Hello, thank you for developing this project! Although I just started to take a look at this, it looks so nice. :smile:

So I'd like to contribute to this project and I believe I can contribute mainly 3 points. How should I contribute? Could I send PRs for these?

1. Release Management

By adding CI scripts like this, if I push a tag,

this CI will run, https://github.com/kenji-miyake/clang-tidy-pr-comments/runs/5168564498?check_suite_focus=true

and a new release is prepared,

and after I publish it, the major version tag is created as well. https://github.com/kenji-miyake/clang-tidy-pr-comments/runs/5168578795?check_suite_focus=true

Why doing so is here: https://github.com/actions/toolkit/blob/2f164000dcd42fb08287824a3bc3030dbed33687/docs/action-versioning.md

2. Other CI scripts

It seems some files are not properly formatted.

By installing pre-commit.ci and other tools, this can be improved and it will reduce your maintenance cost.

Please see this project for more details: https://github.com/autowarefoundation/autoware-github-actions/

3. Bug investigations/fixes

While I'm testing this action, it seems I caught a bug. I can investigate the cause and fix it. https://github.com/kenji-miyake/autoware.universe/runs/5168447074?check_suite_focus=true

-> clang_tidy_fixes should be under the checked-out workspace?

With #38, suggestions including multiple replacements where tried to be tackled. However, right now, replacements text is only concatenated and the length is being summed, which is a bit too simple, resulting in potentially unusable comments.

Consider the following example:

Diagnostics:
- BuildDirectory: /home/runner/work/orbitprofiler/orbitprofiler/build/src/DataViews
  DiagnosticMessage:
    FileOffset: 1269
    FilePath: /home/runner/work/orbitprofiler/orbitprofiler/src/DataViews/CallstackDataView.cpp
    Message: invalid case style for local variable 'resultingColumns_'
    Replacements:
    - FilePath: /home/runner/work/orbitprofiler/orbitprofiler/src/DataViews/CallstackDataView.cpp
      Length: 17
      Offset: 1269
      ReplacementText: resulting_columns
    - FilePath: /home/runner/work/orbitprofiler/orbitprofiler/src/DataViews/CallstackDataView.cpp
      Length: 17
      Offset: 1292
      ReplacementText: resulting_columns
    - FilePath: /home/runner/work/orbitprofiler/orbitprofiler/src/DataViews/CallstackDataView.cpp
      Length: 17
      Offset: 1335
      ReplacementText: resulting_columns
    - FilePath: /home/runner/work/orbitprofiler/orbitprofiler/src/DataViews/CallstackDataView.cpp
      Length: 17
      Offset: 1420
      ReplacementText: resulting_columns
    - FilePath: /home/runner/work/orbitprofiler/orbitprofiler/src/DataViews/CallstackDataView.cpp
      Length: 17
      Offset: 1503
      ReplacementText: resulting_columns
    - FilePath: /home/runner/work/orbitprofiler/orbitprofiler/src/DataViews/CallstackDataView.cpp
      Length: 17
      Offset: 1581
      ReplacementText: resulting_columns
    - FilePath: /home/runner/work/orbitprofiler/orbitprofiler/src/DataViews/CallstackDataView.cpp
      Length: 17
      Offset: 1663
      ReplacementText: resulting_columns
    - FilePath: /home/runner/work/orbitprofiler/orbitprofiler/src/DataViews/CallstackDataView.cpp
      Length: 17
      Offset: 1762
      ReplacementText: resulting_columns
  DiagnosticName: readability-identifier-naming
  Level: Warning
MainSourceFile: ''

Hi,

Might be that this request is not relevant or that the problem lies somewhere else. We (UltiMaker Cura) recently start using your GitHub action, for our custom-made linting tool. We wrote a small Python module which performs linting on a json file which contains definition for 3D printers used in Cura. Since we have a lot of community contributions we want provide the community contributors with some automatic feedback on their PR's

The output of this linting tool follows the same scheme as clang-tidy. See the example below:

Diagnostics:
-   DiagnosticMessage:
        FileOffset: 238
        FilePath: /home/jspijker/cura_wp/Cura/resources/extruders/koonovo_kn5_extruder_1.def.json
        Message: 'Overriding machine_nozzle_offset_x with the same value (0.0) as defined in parent definition: fdmextruder'
        Replacements:
        -   FilePath: /home/jspijker/cura_wp/Cura/resources/extruders/koonovo_kn5_extruder_1.def.json
            Length: 52
            Offset: 246
            ReplacementText: ''
    DiagnosticName: diagnostic-definition-redundant-override
    Level: Warning
-   DiagnosticMessage:
        FileOffset: 299
        FilePath: /home/jspijker/cura_wp/Cura/resources/extruders/koonovo_kn5_extruder_1.def.json
        Message: 'Overriding machine_nozzle_offset_y with the same value (0.0) as defined in parent definition: fdmextruder'
        Replacements:
        -   FilePath: /home/jspijker/cura_wp/Cura/resources/extruders/koonovo_kn5_extruder_1.def.json
            Length: 52
            Offset: 307
            ReplacementText: ''
    DiagnosticName: diagnostic-definition-redundant-override
    Level: Warning
-   DiagnosticMessage:
        FileOffset: 360
        FilePath: /home/jspijker/cura_wp/Cura/resources/extruders/koonovo_kn5_extruder_1.def.json
        Message: 'Overriding machine_nozzle_size with the same value (0.4) as defined in parent definition: fdmextruder'
        Replacements:
        -   FilePath: /home/jspijker/cura_wp/Cura/resources/extruders/koonovo_kn5_extruder_1.def.json
            Length: 48
            Offset: 368
            ReplacementText: ''

However some of our suggested fixes are fixes over multiple lines, which this Action seems to have some trouble with, at least when we use it in conjunction with our custom printer-linter. It could be that the fixes suggested by clang-tidy are always confined to a single line so it isn't an issue for normal usage. But it would really help us if this action handles multi line suggestion comments

The suggestion in the image would result in an ill-formed json

Sometimes I get incorrect suggestions. For example, for this PR the suggestion is:

-	auto findIteratorFor = [](const InterfaceState* state, const Interface& interface) {
+	auto findItfind_iterator_forconst InterfaceState* state, const Interface& interface) {

instead of the following one, which is correctly produced by clang-apply-replacements from the provided fixes file:

-	auto findIteratorFor = [](const InterfaceState* state, const Interface& interface) {
+	auto find_iterator_for = [](const InterfaceState* state, const Interface& interface) {

Looks like the replacement is shifted by 6 characters in the suggestion.

As explained in #17,

• github-release.yaml will prepare the release when a tag is published.
• actions-tagger.yaml will generate a v1/v2/v3 tag when a release is published.

Great project! Are there any real life example project on github which are using this github action? If yes please add them to the readme it would help developers to decide if they want to use it and promote this tool more. Thanks in advance!

Just noticed my actions are failing with the following message, my guess is that it breaks when there are no suggested replacements. I will try to make a minimal example if you need it.

Traceback (most recent call last):
  File "/action/run_action.py", line 4[12], in <module>
    sys.exit(main())
  File "/action/run_action.py", line 268, in main
    suggestion_end = suggestion_begin + diagnostic["ReplacementLength"]
KeyError: 'ReplacementLength'

In this build, clang-tidy found some warnings (and fixes):

Run cat /home/runner/work/industrial_ci/industrial_ci/.work/clang-tidy-fixes.yml
Diagnostics:
- DiagnosticName: readability-identifier-naming
  FileOffset: 26
  FilePath: /home/runner/work/industrial_ci/industrial_ci/.work/target_ws/src/test_clang_tidy/src/test_clang_tidy.cpp
  Message: invalid case style for variable 'a'
  Replacements:
  - {FilePath: /home/runner/work/industrial_ci/industrial_ci/.work/target_ws/src/test_clang_tidy/src/test_clang_tidy.cpp,
    Length: 1, Offset: 26, ReplacementText: A}
MainSourceFile: ''

but the action reports: No warnings found in files changed in this pull request

To start using clang-tidy-pr-comments in a project, I copied the example code in "Using this Action to safely perform analysis of pull requests from forks". I kept getting fatal git errors in git diff -U0 HEAD^.

I found a solution that I hope will be added to the examples to help others. action/[email protected] (and v3) can use fetch-depth: 2 to enable enough context for git diff to work.

One of my test failed with an index out of range error (see logs here). The patches include changes on something like this:

+    // @@@@@@@
+    // test
+    // @@@@@@@
+    /*

And the regex at https://github.com/platisd/clang-tidy-pr-comments/blob/6f016d02636cd7122e972a64220efd8769f9568a/run_action.py#L135-L137 matches this pattern, not giving out diff info, resulting in error. I suggest replacing it with a more strict regex match, maybe like @@ -\d+,\d+ \+\d+,\d+ @@.

In my action, the running log shows that "No warnings found in files changed in this pull request". This error also happened in #17 .

@kenji-miyake All file paths in your fixes.yaml begin with /__w/... (probably because clang-tidy is running self-hosted) while this action expects the GitHub directory layout (/home/runner/...), so it can't normalize paths and is not able to find all these files from your PR. Originally posted by @oleg-derevenetz in https://github.com/platisd/clang-tidy-pr-comments/issues/17#issuecomment-1038393661

Thanks @oleg-derevenetz points out that there is another file system in a custom actions container.

I wonder is there a proper way to avoid modifications on fixes.yaml? Maybe GitHub environment variable GITHUB_WORKSPACE helps, creating a symlink from from /home/runner/... to /__w/... .

https://github.com/platisd/clang-tidy-pr-comments/blob/10dea67599cd21bff5391960fe080aa6fd3a164b/README.md?plain=1#L114-L117

Here, the comment says this is "the number of comments per review", but the variable name gives us "suggestions per comment". I suppose the latter one is actually what you mean.

It would be cool if instead of regular comments, the fixes could be suggested as part of the Checks API.

There's a more informative picture here: https://docs.github.com/en/developers/apps/guides/creating-ci-tests-with-the-checks-api#step-25-updating-the-check-run-with-ci-test-results