Skip to content

fix the bug#2255

Closed
dota17 wants to merge 3 commits intonlohmann:developfrom
dota17:fix_2181
Closed

fix the bug#2255
dota17 wants to merge 3 commits intonlohmann:developfrom
dota17:fix_2181

Conversation

@dota17
Copy link
Contributor

@dota17 dota17 commented Jul 9, 2020

This is a new PR for #2181.
From CI logs, the test SECTION("PR #2181 - regression bug with lvalue") has no error for this new PR.
I just added back the original method and then there would be two methods:

ValueType value(const typename object_t::key_type& key, const ValueType& default_value) const //  added back
...
ValueType value(const typename object_t::key_type& key, ValueType && default_value) const

But to be cautious, maybe some more testcases should be added.

@dota17 dota17 requested a review from nlohmann as a code owner July 9, 2020 13:21
@coveralls
Copy link

coveralls commented Jul 9, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 30dd9bd on dota17:fix_2181 into 4c7bd01 on nlohmann:develop.

Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add a test to cover the added function for JSON pointers.

@3215656174

This comment has been minimized.

@3215656174

This comment has been minimized.

@nlohmann

This comment has been minimized.

@3215656174

This comment has been minimized.

@dota17 dota17 closed this Jul 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants