Skip to content

nlohmann-json: 3.6.1 (new formula) #38257

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

Closed
wants to merge 6 commits into from

Conversation

JarryShaw
Copy link
Contributor

nlohmann/json is a (seems) wildly used JSON
library for C++. The formula is directly migrated
from this Tap.
See: https://github.com/nlohmann/json

@fxcoudert fxcoudert added the new formula PR adds a new formula to Homebrew/homebrew-core label Mar 25, 2019
@SMillerDev
Copy link
Member

Optional dependencies are not acceptable in homebrew-core. I'd suggest just using cmake support.

Replaced optional dependencies with build dependencies
according to Homebrew#38257
@SMillerDev
Copy link
Member

You can drop the caveat now that there's no options anymore. And I don't think all the explanation in the test is required. Other maintainers might disagree though.

Removed redundant caveats.
Keeping verbose test case for now.
@JarryShaw
Copy link
Contributor Author

Ah… my bad 🤦‍♂️
Imma keep the tests for now and wait for u guys to review :)

@SMillerDev SMillerDev added the almost there PR is nearly ready to merge label Mar 26, 2019
Copy link
Contributor Author

@JarryShaw JarryShaw left a comment

Choose a reason for hiding this comment

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

Simplified tests based on review per @fxcoudert .

@fxcoudert
Copy link
Member

@JarryShaw I don't think you've pushed your changes to the test, they do not appear

Simplified tests based on review per @fxcoudert
@JarryShaw
Copy link
Contributor Author

@JarryShaw I don't think you've pushed your changes to the test, they do not appear

Sorry again 🤦‍♂️ not familiar with this GitHub feature

Ahhhhhhhhh… so this time it should work :)
@SMillerDev SMillerDev requested a review from fxcoudert April 16, 2019 12:17
@fxcoudert
Copy link
Member

Thanks @JarryShaw this is looking good!

@fxcoudert fxcoudert added ready to merge PR can be merged once CI is green and removed almost there PR is nearly ready to merge labels Apr 18, 2019
@fxcoudert fxcoudert closed this in 09c7b7a Apr 18, 2019
@lock lock bot added the outdated PR was locked due to age label May 18, 2019
@lock lock bot locked as resolved and limited conversation to collaborators May 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new formula PR adds a new formula to Homebrew/homebrew-core outdated PR was locked due to age ready to merge PR can be merged once CI is green
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants