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

Fix links in README.md #955

Merged
merged 3 commits into from
Feb 6, 2018
Merged

Fix links in README.md #955

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
44b40d7
Fix links in README.md
patrikhuber Feb 3, 2018
ae68801
Changed links from master to develop branch
patrikhuber Feb 4, 2018
ee76436
Change to angle brackets
patrikhuber Feb 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ There are myriads of [JSON](http://json.org) libraries out there, and each may e

- **Intuitive syntax**. In languages such as Python, JSON feels like a first class data type. We used all the operator magic of modern C++ to achieve the same feeling in your code. Check out the [examples below](#examples) and you'll know what I mean.

- **Trivial integration**. Our whole code consists of a single header file [`json.hpp`](https://github.com/nlohmann/json/blob/single_include/nlohmann/json.hpp). That's it. No library, no subproject, no dependencies, no complex build system. The class is written in vanilla C++11. All in all, everything should require no adjustment of your compiler flags or project settings.
- **Trivial integration**. Our whole code consists of a single header file [`json.hpp`](https://github.com/nlohmann/json/blob/master/single_include/nlohmann/json.hpp). That's it. No library, no subproject, no dependencies, no complex build system. The class is written in vanilla C++11. All in all, everything should require no adjustment of your compiler flags or project settings.
Copy link
Owner

Choose a reason for hiding this comment

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

Instead of master, this should be the develop branch. It was my mistake to delete one develop too much before. Please change this.


- **Serious testing**. Our class is heavily [unit-tested](https://github.com/nlohmann/json/blob/master/test/src/unit.cpp) and covers [100%](https://coveralls.io/r/nlohmann/json) of the code, including all exceptional behavior. Furthermore, we checked with [Valgrind](http://valgrind.org) that there are no memory leaks. To maintain high quality, the project is following the [Core Infrastructure Initiative (CII) best practices](https://bestpractices.coreinfrastructure.org/projects/289).

Expand All @@ -58,15 +58,15 @@ See the [contribution guidelines](https://github.com/nlohmann/json/blob/master/.
The single required source, file `json.hpp` is in the `single_include/nlohmann` directory or [released here](https://github.com/nlohmann/json/releases). All you need to do is add

```cpp
#include "json.hpp"
#include "nlohmann/json.hpp"
Copy link
Owner

Choose a reason for hiding this comment

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

I am not sure about this. The paragraph before references the specific file, and the paragraph after that no additional headers are required.

Any opinions on this?

Copy link
Contributor Author

@patrikhuber patrikhuber Feb 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I see your point and kind of agree with it. But I think it would be good to promote consistency and including the file as nlohmann/json.hpp.
Otherwise, for example for a beginner, it would most likely be quite a bit confusing.
I would maybe argue that people who download just the header know what they're doing and "are on their own" with regards to include directory and making sure that the file json.hpp is unique etc. And for everybody else, the readme should show the prefered, "namespaced" way to include the library, via nlohmann/json.hpp.

But I think you can probably make the argument both ways ;-) Happy to change it however you see fit.

Copy link
Owner

Choose a reason for hiding this comment

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

In fact, all examples in doc/examples use #include <nlohmann/json.hpp> and accompanied with a note to compile with -Isingle_include. So the README should be consistent to this, too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we also change the include style? By using angle brackets instead of double quotes?

This is a minor thing, but it could avoid some confusion about the location of the file

Copy link
Contributor Author

@patrikhuber patrikhuber Feb 6, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@theodelrieu I'm personally using angle-brackets for system includes and double quotes for "local"/third-party includes, but I'm happy to stick to the conventions of this repo ;-) Changed!


// for convenience
using json = nlohmann::json;
```

to the files you want to use JSON objects. That's it. Do not forget to set the necessary switches to enable C++11 (e.g., `-std=c++11` for GCC and Clang).

You can further use file [`include/json_fwd.hpp`](https://github.com/nlohmann/json/blob/develop/develop/json_fwd.hpp) for forward-declarations. The installation of json_fwd.hpp (as part of cmake's install step), can be achieved by setting `-DJSON_MultipleHeaders=ON`:
You can further use file [`include/nlohmann/json_fwd.hpp`](https://github.com/nlohmann/json/blob/master/include/nlohmann/json_fwd.hpp) for forward-declarations. The installation of json_fwd.hpp (as part of cmake's install step), can be achieved by setting `-DJSON_MultipleHeaders=ON`:
Copy link
Owner

Choose a reason for hiding this comment

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

Again, please use the develop branch.


### Package Managers

Expand Down