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

Clean up and document project files #4560

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Clean up and document project files #4560

wants to merge 2 commits into from
+2,453 −866

Conversation

nlohmann
Copy link
Owner

@nlohmann nlohmann commented Dec 18, 2024 •
edited
Loading

  • update GitHub community documents
    • update code of conduct
    • update contribution guidelines
    • update pull request template
    • update security policy
  • document non-code files in FILES.md
  • add community section to documentation
    • governance (new)
    • code of conduct (link)
    • contribution guidelines (link)
    • security policy (link)
    • quality assurance (new)
    • architecture (new, draft)
  • add DCO requirement
  • update GitHub actions
    • migrate stalebot to GitHub action
    • remove deprecated .lgtm.yml file
    • add CI steps for C++26
    • add Xcode 16.2 (macOS) and Clang 3.4 (Ubuntu) action
    • simplify Ubuntu workflow to use ubuntu-latest more often and reduce dependencies to more exotic containers
  • overwork documentation
  • overwork README
    • move package manager section to documentation
    • compilers use in the CI to documentation
    • add quick reference to README
  • overwork package manager documentation
    • add Swift Package Manager and xmake
    • add example for Conda
    • add overview with logos
    • annotate Buckaroo, CocoaPods, and wsjcpp as outdated
  • cleanup
    • remove .pre-commit-config.yml file
    • pull update for tests/thirdparty/imapdl/filterbr.py
    • move compiler flags to separate CMake files

@nlohmann nlohmann marked this pull request as draft December 18, 2024 21:56
@coveralls
Copy link

Coverage Status

coverage: 99.639%. remained the same
when pulling 8c2821a on cleanup
into 58f5f25 on develop.

@nlohmann nlohmann marked this pull request as ready for review December 20, 2024 14:42
@nlohmann nlohmann added the review needed It would be great if someone could review the proposed changes. label Dec 20, 2024
@nlohmann nlohmann mentioned this pull request Dec 21, 2024
Merged
4 tasks
@nlohmann nlohmann force-pushed the cleanup branch 4 times, most recently from 8a2a368 to 8e98561 Compare January 3, 2025 15:09
@nlohmann nlohmann mentioned this pull request Jan 3, 2025
Merged
4 tasks
nikhilreddydev
nikhilreddydev reviewed Jan 3, 2025
View reviewed changes
README.md
- Those methods **MUST** be available (e.g., proper headers must be included) everywhere you use these conversions. Look at [issue 1108](https://github.com/nlohmann/json/issues/1108) for errors that may occur otherwise.
- When using `template get<your_type>()`, `your_type` **MUST** be [DefaultConstructible](https://en.cppreference.com/w/cpp/named_req/DefaultConstructible). (There is a way to bypass this requirement described later.)
- In function `from_json`, use function [`at()`](https://json.nlohmann.me/api/basic_json/at/) to access the object values rather than `operator[]`. In case a key does not exist, `at` throws an exception that you can handle, whereas `operator[]` exhibits undefined behavior.
- You do not need to add serializers or deserializers for STL types like `std::vector`: the library already implements these.

#### Simplify your life with macros

Copy link
Contributor

@nikhilreddydev nikhilreddydev Jan 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Hello @nlohmann
As mentioned in #4260
I think README file reads incorrectly here as two macros, as there are now 6 of them.

Or Is it fine as they are actually 2 types of macros (intrusive & non-intrusive) and as an improvement we can just add links to the documentation for further information?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks for the comment. As I wrote in #4582 (review) I will shorten this part of the README in a later commit.

@nlohmann
🔧 clean up project
e4b3abd 
Signed-off-by: Niels Lohmann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikhilreddydev nikhilreddydev nikhilreddydev left review comments

Assignees
No one assigned
Labels
L review needed It would be great if someone could review the proposed changes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nlohmann @coveralls @nikhilreddydev