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

Add a note about installing the library with cget #954

Merged
merged 3 commits into from
Feb 9, 2018

Conversation

pfultz2
Copy link
Contributor

@pfultz2 pfultz2 commented Feb 2, 2018

This adds a note about installing with cget since its already supported(due to supporting cmake installation out of the box).

@coveralls
Copy link

coveralls commented Feb 2, 2018

Coverage Status

Coverage remained the same at ?% when pulling 5731695 on pfultz2:patch-1 into 3a887dc on nlohmann:develop.

@pfultz2
Copy link
Contributor Author

pfultz2 commented Feb 4, 2018

The failures on travis seems unrelated to this.

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.

  • "the latest" -> "the latest version"
  • "(ie cget install nlohmann/json -DJSON_MultipleHeaders=ON)" -> "(i.e., cget install nlohmann/json -DJSON_MultipleHeaders=ON)"

@nlohmann
Copy link
Owner

nlohmann commented Feb 9, 2018

@pfultz2 Could you address the requested changes, please?

@pfultz2
Copy link
Contributor Author

pfultz2 commented Feb 9, 2018

"the latest" -> "the latest version"

This actually installs the latest from develop, so it is the latest version, but not the latest released version.

@nlohmann
Copy link
Owner

nlohmann commented Feb 9, 2018

I just made that comment because the previous sentence missed a word. Maybe "the latest development version"? Furthermore, there is a space missing after "flag".

@pfultz2
Copy link
Contributor Author

pfultz2 commented Feb 9, 2018

I just made that comment because the previous sentence missed a word. Maybe "the latest development version"?

Sure, that works.

Furthermore, there is a space missing after "flag".

Got it.

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 Feb 9, 2018
@nlohmann nlohmann added this to the Release 3.1.1 milestone Feb 9, 2018
@nlohmann nlohmann merged commit e5d538c into nlohmann:develop Feb 9, 2018
@nlohmann
Copy link
Owner

nlohmann commented Feb 9, 2018

Thanks!

nlohmann added a commit that referenced this pull request Feb 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants