Skip to content

Properly constrain non-string json_pointer overloads #3681

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Aug 7, 2022
Merged

Properly constrain non-string json_pointer overloads #3681

merged 1 commit into from
Aug 7, 2022

Conversation

falbrechtskirchinger
Copy link
Contributor

Add missing is_basic_json constraints to the deprecated non-string json_pointer overloads.

Thanks, @alex-700, for reminding me.

Compared to #3658, the value() functions and the regression test have been omitted. Both are handled in #3663.

Closes #3658.

@nlohmann
Copy link
Owner

nlohmann commented Aug 5, 2022

In which order should this be merged? After #3663?

@github-actions github-actions bot added the M label Aug 5, 2022
@coveralls
Copy link

coveralls commented Aug 5, 2022 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling 8cc150c on falbrechtskirchinger:fix-constraints into b0422f8 on nlohmann:develop.

@falbrechtskirchinger
Copy link
Contributor Author

Shouldn't matter since this one doesn't touch value(). I'd expect it to merge without conflicts.

falbrechtskirchinger
falbrechtskirchinger commented Aug 6, 2022
View reviewed changes
falbrechtskirchinger
falbrechtskirchinger commented Aug 6, 2022
View reviewed changes
falbrechtskirchinger
falbrechtskirchinger commented Aug 7, 2022
View reviewed changes
Copy link
Contributor Author

@falbrechtskirchinger falbrechtskirchinger left a comment

Choose a reason for hiding this comment

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

Looks good to me.

nlohmann
nlohmann approved these changes Aug 7, 2022
View reviewed changes
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.

Looks good to me.

@nlohmann nlohmann added this to the Release 3.11.2 milestone Aug 7, 2022
@nlohmann nlohmann merged commit dae5412 into nlohmann:develop Aug 7, 2022
@falbrechtskirchinger falbrechtskirchinger deleted the fix-constraints branch August 7, 2022 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nlohmann nlohmann nlohmann approved these changes

+1 more reviewer

@shaischaudhry shaischaudhry shaischaudhry approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
M
Projects
None yet
Milestone
Release 3.11.2
Development

Successfully merging this pull request may close these issues.
4 participants
@falbrechtskirchinger @nlohmann @coveralls @shaischaudhry