Skip to content

Conversation

nlohmann
Copy link
Owner

@nlohmann nlohmann commented Nov 9, 2024

This PR fixes a lot of warnings from Clang-Tidy (or suppresses them in case of false positives). Furthermore, GitHub actions have been updated.

Details:

Clang-Tidy warnings

The following warnings are currently suppressed to have a green CI. They need to be addressed at some point:

-portability-template-virtual-member-function,
-bugprone-use-after-move,
-hicpp-invalid-access-moved,

Suppressed the following warnings:

  • boost-use-ranges: we don't want to rely on Boost
  • modernize-use-designated-initializers: designated initializers are a C++20 feature
  • modernize-use-ranges: ranges are a C++20 feature

Update GitHub actions

  • update all workflows to use the newer versions actions/upload-artifact@v4, actions/checkout@v4, github/codeql-action/init@v3, peaceiris/actions-gh-pages@v4
  • remove unsupported macos-11 image; the following Xcode versions are no longer tested in the CI: '11.7', '12.4', '12.5.1', '13.0'
  • add macos-13 image to test Xcode versions '14.2', '14.3', '14.3.1', '15.0.1', '15.1', '15.2'
  • add macos-14 image to test Xcode versions '15.3', '15.4'
  • add macos-15 image to test Xcode versions '16.0', '16.1'
  • test GCC versions '4.8', '4.9', '5', '6' on json-ci image as these versions are no longer work on the respective gcc images (checkout fails)
  • add tests for GCC versions '13', '14'
  • add tests for Clang versions '16', '17', '18', '19'
  • update REAMDE with table of compilers/OS used in the CI

@github-actions github-actions bot added the CI label Nov 9, 2024
@coveralls
Copy link

coveralls commented Nov 9, 2024

Coverage Status

coverage: 100.0%. remained the same
when pulling 4658117 on fix_ci_part_10000
into 18ff442 on develop.

@nlohmann nlohmann mentioned this pull request Nov 10, 2024
@nlohmann nlohmann marked this pull request as ready for review November 10, 2024 12:10
@nlohmann nlohmann added the review needed It would be great if someone could review the proposed changes. label Nov 10, 2024
Copy link
Contributor

@fsandhei fsandhei left a comment

Choose a reason for hiding this comment

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

Code-wise, I think it looks good! Can't say anything about the compiler supports.

Just one question: When disabling lints, do you have a norm to give a reasoning on why some lints are disabled? I find that it helps to have some comment next to the NOLINT statement to remind me when I inevitable forget why I disabled this lint.

@nlohmann
Copy link
Owner Author

Code-wise, I think it looks good! Can't say anything about the compiler supports.

Thanks!

Just one question: When disabling lints, do you have a norm to give a reasoning on why some lints are disabled? I find that it helps to have some comment next to the NOLINT statement to remind me when I inevitable forget why I disabled this lint.

Yes, I agree. There are 134 NOLINTs in the code, and in a perfect world, most of them should be annotated. I'll have a look, though probably not in this PR...

@nlohmann
Copy link
Owner Author

I merge this now. Good is better than perfect...

@nlohmann nlohmann added this to the Release 3.11.4 milestone Nov 13, 2024
@nlohmann nlohmann merged commit 1825117 into develop Nov 13, 2024
123 checks passed
@nlohmann nlohmann deleted the fix_ci_part_10000 branch November 13, 2024 09:21
slowriot pushed a commit to slowriot/json that referenced this pull request Jan 10, 2025
* 🚨 fix warning

* 💚 update actions

* 🚨 fix warning

* 🚨 fix warning

* 🚨 fix warning

* 💚 update actions

* 💚 update actions

* 🚨 fix warning

* 🚨 fix warning

* 💚 update actions

* 🚨 fix warning

* 💚 update actions

* 💚 update actions

* 💚 update actions

* 🚨 fix warning

* 🚨 fix warning

* 🚨 fix warning

* 🚨 fix warning

* 💚 update actions

* 💚 update actions

* 🚨 fix warning

* 💚 update actions

* 💚 update actions

* 💚 update actions

* 💚 update actions

* 💚 update actions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI documentation L review needed It would be great if someone could review the proposed changes. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants