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.

Sign up for GitHub

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

Jump to bottom

release: add singleinclude and meson.build to include.zip #1694

Merged
merged 2 commits into from
Nov 2, 2019
Merged

release: add singleinclude and meson.build to include.zip #1694

merged 2 commits into from
Nov 2, 2019

Conversation

eli-schwartz
Copy link
Contributor

@eli-schwartz eli-schwartz commented Jul 31, 2019 •
edited
Loading

This serves as a minimal release-only way to embed json into a project. Add meson support to this directly, to make it usable standalone as ameson subproject.

Fixes #1672

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 file single_include/nlohmann/json.hpp. The whole process is described here.

@coveralls
Copy link

coveralls commented Jul 31, 2019 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling f4332d4 on eli-schwartz:release-include-meson into 0245ae5 on nlohmann:develop.

@stale
Copy link

stale bot commented Aug 30, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Aug 30, 2019
@eli-schwartz
Copy link
Contributor Author

Still waiting for a review. ;)

@stale stale bot removed the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Aug 30, 2019
nlohmann
nlohmann approved these changes Sep 2, 2019
View reviewed changes
Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@nlohmann nlohmann self-assigned this Sep 2, 2019
@nlohmann nlohmann added this to the Release 3.7.1 milestone Sep 2, 2019
@nlohmann
Copy link
Owner

nlohmann commented Sep 2, 2019

Before I merge this, I need to understand the role of this change. I am not using Meson myself, so I do not know whether it is actually needed. The README states:

If you are using the Meson Build System, then you can get a wrap file by downloading it from Meson WrapDB, or simply use meson wrap install nlohmann_json.

Why do we need this change here in this repository?

@eli-schwartz
Copy link
Contributor Author

That is covered by the second point from #1672

@nlohmann
Copy link
Owner

nlohmann commented Sep 2, 2019

I see. Though it may be unwise to rely on third parties for Meson, I will also not be able to maintain the package. Could you add another sentence to the README describing how to still use the library with Meson if you do not use the provided package?

@eli-schwartz
Copy link
Contributor Author

I added a couple blurbs to the README, including a mention of pkg-config. Since meson's documentation exposes a preference for pkg-config, and the meson documentation on subprojects makes reference to pkg-config dependencies with subproject fallbacks, meson users will likely be interested in looking for pkg-config support. So the pkg-config blurb lets them know that pkg-config support can be available, and thus nlohmann_json is eligible to use that dependency fallback stuff.

But from a simple perspective, it is as simple as "use this source tree" or "use the include.zip which is equal to this subtree minus a lot of non-release files".

@eli-schwartz
Copy link
Contributor Author

Okay, force pushed to include Table of Contents links to the new sections I added.

nlohmann
nlohmann requested changes Sep 10, 2019
View reviewed changes
Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

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

I do not think the Meson documentation should be so prominent. Could you please move it to the "Package Managers" section?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@stale
Copy link

stale bot commented Oct 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Oct 10, 2019
@stale stale bot closed this Oct 17, 2019
@eli-schwartz
Copy link
Contributor Author

@nlohmann What happened?

@nlohmann nlohmann reopened this Oct 17, 2019
@stale stale bot removed the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Oct 17, 2019
nlohmann
nlohmann reviewed Oct 17, 2019
View reviewed changes
Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

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

Please have a look at the comments.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@eli-schwartz
release: add singleinclude and meson.build to include.zip
84faa36 
This serves as a minimal release-only way to embed json into a project.
Add meson support to this directly, to make it usable standalone as a
meson subproject.

Implements #1672
@eli-schwartz
README: describe how to use json as a meson subproject
f4332d4 
Also call out to the guidelines for using pkg-config dependencies first,
and reference it for other build systems as well.

Although the possibility of installing with a pkg-config file is
somewhat hidden away in the meson docs, it's been deemed less invasive
due to not distracting away from cmake. So it will have to do.
@eli-schwartz
Copy link
Contributor Author

Okay, hopefully this should be good for now.

nlohmann
nlohmann approved these changes Nov 2, 2019
View reviewed changes
Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@nlohmann nlohmann merged commit 1307862 into nlohmann:develop Nov 2, 2019
@nlohmann
Copy link
Owner

nlohmann commented Nov 2, 2019

🔖 Release item

This issue/PR will be part of the next release of the library. This template helps preparing the release notes.

Type

  • ✨ New Feature
  • 🐛 Bug Fix
  • ⚡️ Improvement
  • 🔨 Further Change
  • 🔥 Deprecated function

Description

  • add singleinclude and meson.build to include.zip

@nlohmann
Copy link
Owner

nlohmann commented Nov 2, 2019

Thanks!

@eli-schwartz eli-schwartz deleted the release-include-meson branch November 3, 2019 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nlohmann nlohmann nlohmann approved these changes

Assignees

@nlohmann nlohmann

Labels
release item: 🔨 further change
Projects
None yet
Milestone
Release 3.7.1
Development

Successfully merging this pull request may close these issues.

include.zip should contain meson.build
3 participants
@eli-schwartz @coveralls @nlohmann