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

Add migration guide #3702

Closed
wants to merge 61 commits into from
Closed

Add migration guide #3702

wants to merge 61 commits into from

Conversation

nlohmann
Copy link
Owner

The migration guide collects all steps required to make the code future-proof for upcoming releases.

@nlohmann nlohmann marked this pull request as draft August 14, 2022 11:14
@coveralls
Copy link

coveralls commented Aug 14, 2022

Coverage Status

Coverage remained the same at 100.0% when pulling ac6adbf on migration_guide into 8fcdbf2 on develop.

docs/mkdocs/docs/integration/migration_guide.md Outdated Show resolved Hide resolved
docs/mkdocs/docs/integration/migration_guide.md Outdated Show resolved Hide resolved
[`operator!=`](../api/json_pointer/operator_ne.md) is deprecated since 3.11.2. To compare a
[`json_pointer`](../api/json_pointer/index.md) `p` with a string `s`, convert `s` to a `json_pointer` first and use
[`json_pointer::operator==`](../api/json_pointer/operator_eq.md) or
[`json_pointer::operator!=`](../api/json_pointer/operator_ne.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

This may be confusing.

The member operator== does not replace the non-member operators due to deprecation but due to the used C++ standard version.

Until C++20 the non-member operators operator==(json_pointer, string_t) and operator==(string_t, json_pointer) are deprecated for operator==(json_pointer, json_pointer). (Same applies to operator!=.)

Since C++20 the member operator json_pointer::operator==(string_t) is deprecated for json_pointer::operator==(json_pointer). json_pointer::operator!= does not exist in the source code but is synthesized by the compiler by rewriting the expression using json_pointer::operator==, i.e., a == b is rewritten as !(a == b).

This is by no means necessary but could reduce code size minimally. (Someone – me? – should verify that on Compiler Explorer.)

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm also confused. Can you propose a better wording for the paragraph?

docs/mkdocs/docs/integration/migration_guide.md Outdated Show resolved Hide resolved
Comment on lines 211 to 216
## Do not hard-code the complete library namespace

The [`nlohmann` namespace](../features/namespace.md) contains two sub-namespaces to avoid problems when different
versions or configurations of the library are used in the same project. Always use `nlohmann` as namespace or, when the
exact version and configuration is relevant, use macro
[`NLOHMANN_JSON_NAMESPACE`](../api/macros/nlohmann_json_namespace.md) to denote the namespace.
Copy link
Contributor

Choose a reason for hiding this comment

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

This section is tricky. I keep oscillating on what is necessary and when.

I'll expand on this later.

(Also, "two sub-namespaces" is definitely wrong now.)

Copy link
Owner Author

Choose a reason for hiding this comment

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

I fixed the "two sub-namespaces". No idea about the rest.

Copy link
Contributor

Choose a reason for hiding this comment

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

I hesitate because of the advice given here:
https://json.nlohmann.me/features/arbitrary_types/#how-do-i-convert-third-party-types

Currently, I think this section is OK (i.e., the issue is resolved for the purpose of this PR) but I'm conflicted about the advice to use NLOHMAN_JSON_NAMESPACE_BEGIN when specializing adl_serializer.

Co-authored-by: Florian Albrechtskirchinger <falbrechtskirchinger@gmail.com>
@github-actions github-actions bot removed the CI label Aug 17, 2022
@nlohmann
Copy link
Owner Author

What do you think of a preprocessor symbol like JSON_PEDANTIC or JSON_FUTURE_PROOF that removes (or adds #error directives to) all deprecated functions or discouraged behavior like implicit conversions? This could be a mode that people could use to have a rough idea whether their code would be still usable in a future version?

@gregmarr
Copy link
Contributor

That sounds like a good idea. Maybe a JSON_REMOVE_DEPRECATED_FUNCTIONS for the functions, and then a single define that sets all of the macros that remove deprecated behaviors, such as implicit conversions and the compare behavior.

@nlohmann
Copy link
Owner Author

@nlohmann
Copy link
Owner Author

That sounds like a good idea. Maybe a JSON_REMOVE_DEPRECATED_FUNCTIONS for the functions, and then a single define that sets all of the macros that remove deprecated behaviors, such as implicit conversions and the compare behavior.

I will not do this in this PR.

falbrechtskirchinger and others added 9 commits August 27, 2022 14:20
Co-authored-by: Niels Lohmann <niels.lohmann@gmail.com>
Co-authored-by: Florian Albrechtskirchinger <falbrechtskirchinger@gmail.com>
Co-authored-by: barcode <barcode@example.com>
* 💚 add clang-tools to required tools for ci_static_analysis_clang

* 🚨 update Clang-Tidy warning selection

* 🚨 fix Clang-Tidy warnings

* 🚨 fix Clang-Tidy warnings

* 🚨 fix Clang-Tidy warnings

* 🚨 fix Clang-Tidy warnings

* 🚨 fix Clang-Tidy warnings

* 🚨 fix Clang-Tidy warnings

* 🚨 fix Clang-Tidy warnings

* 🚨 fix Clang-Tidy warnings

* 🚨 fix Clang-Tidy warnings

* 🚨 fix Clang-Tidy warnings

* 🚨 fix Clang-Tidy warnings (#3738)

* ⏪ revert fix

* ⏪ revert fix

* 🚨 fix Clang-Tidy warnings (#3739)

Co-authored-by: Florian Albrechtskirchinger <falbrechtskirchinger@gmail.com>
@nlohmann nlohmann mentioned this pull request Dec 18, 2022
@nlohmann nlohmann closed this Dec 18, 2022
@nlohmann nlohmann deleted the migration_guide branch December 18, 2022 16:04
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.

9 participants