search

nlohmann / json

JSON for Modern C++
https://json.nlohmann.me
MIT License
41.25k stars 6.57k forks source link

Add lgtm explanation #4362

Open nlohmann opened 2 months ago

nlohmann commented 2 months ago

Closes #4361 by adding a comment why goto should not be considered harmful in the number parser.

coveralls commented 2 months ago

Coverage Status

coverage: 100.0%. remained the same when pulling 2785437d0e03f0fba39a09178d2f8766e8556863 on lgtm-goto-explanation into 8c391e04fe4195d8be862c97f38cfe10e2a3472e on develop.

TheJCAB commented 2 months ago

Thanx!

Note that I don't know if a justification in the old style suppression format will be accepted. The internal guide to resolve the issue just mentions the "// CodeQL [query-id] Justification" format, which IIUC needs to be in a comment on its own line (not appended to the code). From my searches in the codeql public documentation, the only mention of suppressions (old or new) is in the release notes of CodeQL 2.12.0 announcing support for the new format, and from looking at documentation for the CodeQL scanning offered here on GitHub, they seem to be using 2.17.0, so that should work fine. I'd encourage you to try it.

TheJCAB commented 1 month ago

I opened support requests on both CodeQL here on github and on the internal team managing CodeQL policies, but I've heard nothing from either.

I say go ahead and complete this as-is. It's an improvement and I expect it will suffice. If changing the suppression style becomes needed, it'll be a problem for another day.

Thank you!