checkpoint-restore / criu

Checkpoint/Restore tool
criu.org
Other
2.79k stars 565 forks source link

Lint add warnings instead of check failure #2250

Closed Snorch closed 11 months ago

Snorch commented 11 months ago

There are multiple cases where good human readable code block is converted to an unreadable mess by clang-format, so we don't want to rely on clang-format completely. Also there is no way, as far as I can see, to make clang-format only fix what we want it to fix without breaking something.

So let's just display hints inline where clang-format is unhappy. When reviewer sees such a warning it's a good sign that something is broken in codding style around this warning.

Snorch commented 11 months ago

It looks like that:

image

Please remove draft/test: introduce some codding style errors to test new approach when merging this PR, this commit is needed just to illustrate the concept.

codecov-commenter commented 11 months ago

Codecov Report

Patch coverage: 84.61% and project coverage change: +0.06% :tada:

Comparison is base (5fedcaa) 70.62% compared to head (6ce24a5) 70.68%. Report is 14 commits behind head on criu-dev.

:exclamation: Current head 6ce24a5 differs from pull request most recent head 99d878b. Consider uploading reports for the commit 99d878b to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## criu-dev #2250 +/- ## ============================================ + Coverage 70.62% 70.68% +0.06% ============================================ Files 133 133 Lines 33322 33328 +6 ============================================ + Hits 23532 23557 +25 + Misses 9790 9771 -19 ``` | [Files Changed](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2250?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=checkpoint-restore) | Coverage Δ | | |---|---|---| | [criu/proc\_parse.c](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2250?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=checkpoint-restore#diff-Y3JpdS9wcm9jX3BhcnNlLmM=) | `68.54% <50.00%> (-0.03%)` | :arrow_down: | | [criu/memfd.c](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2250?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=checkpoint-restore#diff-Y3JpdS9tZW1mZC5j) | `82.77% <85.71%> (+1.00%)` | :arrow_up: | | [compel/src/lib/infect.c](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2250?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=checkpoint-restore#diff-Y29tcGVsL3NyYy9saWIvaW5mZWN0LmM=) | `54.67% <100.00%> (+0.18%)` | :arrow_up: | | [criu/files-reg.c](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2250?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=checkpoint-restore#diff-Y3JpdS9maWxlcy1yZWcuYw==) | `75.38% <100.00%> (+0.02%)` | :arrow_up: | ... and [5 files with indirect coverage changes](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2250/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=checkpoint-restore)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Snorch commented 11 months ago

I've added changes to CONTRIBUTING.md with bad clang-formating examples to PR and a link from warnings to CONTRIBUTING.md, so that it would be easier to understand how this warning should be handled.

Snorch commented 11 months ago

I've removed test commit to trigger warnings (it works), so it's now ready to merge.