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

fix jwt-cpp on_test #5366

Merged
merged 3 commits into from
Sep 29, 2024
Merged

fix jwt-cpp on_test #5366

merged 3 commits into from
Sep 29, 2024

Conversation

TTopoo
Copy link
Contributor

@TTopoo TTopoo commented Sep 28, 2024

check package:config("picojson") before on_test

]]}, {configs = {languages = "c++11"}}))
if package:config("picojson") then
assert(package:check_cxxsnippets({test = [[
#include <jwt-cpp/jwt.h>
Copy link
Member

Choose a reason for hiding this comment

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

Could you please provide more information about this change? When the picojson feature is disabled with this patch, it will be completely untested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image
If I use add_requires("jwt-cpp", {configs = {picojson = false}}), the situation shown in the figure will appear when installing the package

Copy link
Member

Choose a reason for hiding this comment

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

I understand. In this case, the current port of the package can be enhanced by incorporating additional JSON trait features, such as Boost.JSON or nlohmann_json. These dependencies should be added when the feature is enabled. Otherwise, simply disabling the on_test won't be very effective.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I use nlohmann-json by add_requires in my project code, I just want to be able to install jwt after disabling picojson.
If the programmer actively turns off this picojson config, it means that he is consciously using other json libraries.

Copy link
Contributor

Choose a reason for hiding this comment

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

Need refactor the package

@star-hengxing
Copy link
Contributor

No issue? I merged

@star-hengxing star-hengxing merged commit 0264111 into xmake-io:dev Sep 29, 2024
67 checks passed
@TTopoo TTopoo deleted the patch-1 branch September 29, 2024 12:05
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.

3 participants