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

[202305] Use json.hpp from nlohmann-json-dev instead of from swss-common (#2900) #2991

Open
wants to merge 1 commit into
base: 202305
Choose a base branch
from

Conversation

kcudnik
Copy link
Contributor

@kcudnik kcudnik commented Dec 13, 2023

  • Use json.hpp from nlohmann-json-dev instead of from swss-common

This header file comes from an external package, and a very old version of the header file has been checked into swss-common. This will cause problems for the upcoming Bookworm upgrade.

Change references to the header file to use the Debian package nlohmann-json-dev, instead of from swss-common.

What I did

Why I did it

How I verified it

Details if related

…c-net#2900)

* Use json.hpp from nlohmann-json-dev instead of from swss-common

This header file comes from an external package, and a very old version of the
header file has been checked into swss-common. This will cause problems for the
upcoming Bookworm upgrade.

Change references to the header file to use the Debian package
nlohmann-json-dev, instead of from swss-common.

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
@saiarcot895
Copy link
Contributor

I thought the plan was to change sairedis 202305 branch to not use json.hpp from nlohmann-json-dev? Trying to move everything to use nlohmann-json-dev will be a bigger effort that should not happen on a release branch.

@kcudnik
Copy link
Contributor Author

kcudnik commented Dec 13, 2023

I thought the plan was to change sairedis 202305 branch to not use json.hpp from nlohmann-json-dev? Trying to move everything to use nlohmann-json-dev will be a bigger effort that should not happen on a release branch.

this change is just cherrypick from master, so we want to move in that direction, why this will be a problem ?

@saiarcot895
Copy link
Contributor

Two reasons:

  1. Cherry-picks/backports are meant for necessary bug fixes; this backport (of using json.hpp from the nlohmann-json-dev package instead of the local copy in swss-common) is not a required bug fix for 202305, besides the fact that a very old version is being used here.
  2. There are multiple submodules that use json.hpp from swss-common. With this, there is now more of a mix of code with some components using json.hpp from swss-common and some components using json.hpp from nlohmann-json-dev. Following this through will mean a bigger effort that opens the door to possible bugs than just using the existing json.hpp from swss-common.

@prsunny prsunny changed the title Use json.hpp from nlohmann-json-dev instead of from swss-common (#2900) [202305] Use json.hpp from nlohmann-json-dev instead of from swss-common (#2900) Dec 13, 2023
@prsunny
Copy link
Collaborator

prsunny commented Dec 13, 2023

@kcudnik , please link the master PR if you are cherry-picking

@kcudnik
Copy link
Contributor Author

kcudnik commented Dec 13, 2023

@kcudnik , please link the master PR if you are cherry-picking

number is in this PR name :D #2900

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.

None yet

3 participants