-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
Fix links in README.md #955
Conversation
README.md
Outdated
@@ -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. |
There was a problem hiding this comment.
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.
README.md
Outdated
@@ -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" |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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!
README.md
Outdated
|
||
// 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`: |
There was a problem hiding this comment.
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.
There was a problem hiding this 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.
These two links in the README were broken after the latest include directory changes.
I wasn't sure whether you want them all to link to the
develop
ormaster
branch, since thedevelop
branch is the default of the repository, and some links in the readme point tomaster
and some todevelop
.