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

feat: nlohmann_json add single_include package #2865

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

Conversation

lromor
Copy link

@lromor lromor commented Sep 30, 2024

nlohmann_json provides a new single-header target. Add it as optional target to the the bazel module.

@bazel-io
Copy link
Member

Hello @keith, modules you maintain (nlohmann_json) have been updated in this PR. Please review the changes.

@lromor lromor changed the title feat: nlohmann_json add single-header as default target feat: nlohmann_json add single-header as new target Oct 1, 2024
@lromor lromor force-pushed the add-nlohmann-json-multi-header branch 6 times, most recently from 25ff0a3 to 106bfb5 Compare October 1, 2024 08:33
@lromor lromor changed the title feat: nlohmann_json add single-header as new target feat: nlohmann_json add single_include package Oct 1, 2024
@hzeller
Copy link

hzeller commented Oct 1, 2024

I'd put it in the toplevel BUILD file. Keep the original :json and also provide a :singleheader/json

Reasoning: for projects that also support the 'old' WORKSPACE file, they have a simple way to provide a toplevel BUILD file, but not a nested structure. A project that needs to compile with before and after bzlmod should be able to do that easily.

Example of such a project and how it will be working nicely with a toplevel target : chipsalliance/verible#2268

@lromor lromor force-pushed the add-nlohmann-json-multi-header branch from 106bfb5 to 7b71bd4 Compare October 1, 2024 16:51
@lromor lromor force-pushed the add-nlohmann-json-multi-header branch from 7b71bd4 to a26590b Compare October 1, 2024 16:53
@lromor lromor marked this pull request as ready for review October 1, 2024 16:58
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