Skip to content
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.

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

Clean up custom base class #3899

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

barcode
Copy link
Contributor

@barcode barcode commented Dec 26, 2022

(Second part of the points listed in #3110 (comment), the rest is done in a different PR to prevent mixing of separate issues / features)

  • Warn users about potential conflicts with newer versions, i.e., base members should avoid generic names to avoid clashes with future member additions to basic_json.
  • (Run reuse.)

Also adds a new method as_base_class to make it easier to access members of a custom base class shadowed by nlohmann::basic_json


Pull request checklist

Read the Contribution Guidelines for detailed information.

  • Changes are described in the pull request, or an existing issue is referenced.
  • The test suite compiles and runs without error.
  • Code coverage is 100%. Test cases can be added by editing the test suite.
  • The source code is amalgamated; that is, after making changes to the sources in the include/nlohmann directory, run make amalgamate to create the single-header files single_include/nlohmann/json.hpp and single_include/nlohmann/json_fwd.hpp. The whole process is described here.

@coveralls
Copy link

coveralls commented Dec 26, 2022

Coverage Status

coverage: 100.0%. remained the same when pulling 0f6d471 on barcode:clean_up_custom_base_class into 5d27543 on nlohmann:develop.

@barcode
Copy link
Contributor Author

barcode commented Feb 1, 2023

Work on this PR is done and it only needs a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants