How to contribute to this project?

[kenji-miyake]

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

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?

[kenji-miyake]

@platisd Hi, could you take a look at this, please? 🙏

[platisd]

Hi @kenji-miyake, all three points sound like great ways to contribute, even though I haven't yet fully understood the last point. 👍
Any PR are totally welcome and thanks for your interest. :)

[oleg-derevenetz]

Hi @platisd

even though I haven't yet fully understood the last point

Since this action is executed in Docker container, it has its own /tmp, not the system /tmp. fixes.yml should be created somewhere in the $HOME, because it's mounted inside the Docker image, while system /tmp does not. This is not a bug in this action, but just in the caller workflow.

[kenji-miyake]

@platisd Thank you, I'll send PRs and explain more details. Let's discuss there.

@oleg-derevenetz Thank you for your explanation!
Yes, you are right, only a few directories will be mounted. After I copied the file under the workspace, it worked.

This was due to my lack of understanding, but I feel it's nice to write it in the README.md, because somebody would try to avoid "Redownload" as I did.

jobs:
  clang-tidy-pr-comments:
    if: ${{ github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.conclusion == 'success' }}
    runs-on: ubuntu-latest
    steps:
      - name: Check out repository
        uses: actions/checkout@v2

      - name: Download analysis results
        run: |
          gh run download ${{ github.event.workflow_run.id }} -D /tmp
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

      - name: Set variables
        id: set-variables
        run: |
          echo ::set-output name=pr-id::"$(cat /tmp/clang-tidy-result/pr-id.txt)"
          echo ::set-output name=pr-head-repo::"$(cat /tmp/clang-tidy-result/pr-head-repo.txt)"
          echo ::set-output name=pr-head-ref::"$(cat /tmp/clang-tidy-result/pr-head-ref.txt)"
      - name: Check out PR head
        uses: actions/checkout@v2
        with:
          repository: ${{ steps.set-variables.outputs.pr-head-repo }}
          ref: ${{ steps.set-variables.outputs.pr-head-ref }}
          persist-credentials: false

      - name: Copy fixes.yaml to access from Docker Container Action
        run: |
          cp /tmp/clang-tidy-result/fixes.yaml fixes.yaml

      - name: Run clang-tidy-pr-comments action
        uses: platisd/clang-tidy-pr-comments@1.1.6
        with:
          github_token: ${{ secrets.GITHUB_TOKEN }}
          clang_tidy_fixes: fixes.yaml
          pull_request_id: ${{ steps.set-variables.outputs.pr-id }}

[kenji-miyake]

By the way, are you interested in Conventional Commits?

If we follow this, we can automatically generate the changelog by orhun/git-cliff.
For example, please see this one: tier4/autoware.universe#11 (comment)
It can also be used for GitHub Releases.

To enforce it, we can use amannn/action-semantic-pull-request/.

If you are interested, I can send a PR.

[kenji-miyake]

I sent PRs #18, #19, and #20.
Please take a look at them when you have the time.
If you have any questions, I can explain them in more detail.

[kenji-miyake]

@platisd Thank you for merging PRs!
If possible, could I create an issue for my question or can I ask it here?

I'd like to ask why no warnings were found with this PR.

• PR: test Clang-Tidy kenji-miyake/autoware.universe#2
• Artifacts: https://github.com/kenji-miyake/autoware.universe/actions/runs/1836303338
• Result: https://github.com/kenji-miyake/autoware.universe/runs/5173061895?check_suite_focus=true#step:8:9

The following diagnostic is in `fixes.yaml.

  - DiagnosticName:  modernize-use-using
    DiagnosticMessage:
      Message:         'use ''using'' instead of ''typedef'''
      FilePath:        '/__w/autoware.universe/autoware.universe/common/tier4_autoware_utils/include/tier4_autoware_utils/geometry/geometry.hpp'
      FileOffset:      1328
      Replacements:
        - FilePath:        '/__w/autoware.universe/autoware.universe/common/tier4_autoware_utils/include/tier4_autoware_utils/geometry/geometry.hpp'
          Offset:          1328
          Length:          16
          ReplacementText: 'using B = double'

Should I run run-clang-tidy instead of clang-tidy here? 🤔

[platisd]

Maybe it's best you create a separate issue for this.
I haven't looked more into this yet, but for what it counts I always use clang-tidy via the wrapper script.

[oleg-derevenetz]

@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.

[platisd]

Thanks @oleg-derevenetz for the quick response! 👍

@kenji-miyake I just saw your suggestion about conventional commits. I don't personally use the particular convention and since this project isn't crazy active, I'd say no need to complicate things when committing. :)

[kenji-miyake]

@oleg-derevenetz Thank you! I'll look into it. This result was by a GitHub-hosted runner with composite action.

I'd say no need to complicate things when committing. :)

@platisd Yeah, it's fine. Please recall this if necessary in the future. 😄

And I'll close this issue since my PRs are merged.
Will create other issues if necessary.
Thank you very much, again!

[kenji-miyake]

@oleg-derevenetz By fixing the paths, it worked! Thank you!
https://github.com/kenji-miyake/autoware.universe/blob/1b694388fdaabf32eaa9a9b55c192c7694986890/.github/workflows/clang-tidy-pr-comments.yaml#L42

[caic99]

@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.

Hi @oleg-derevenetz ,
I'm facing exactly the same problem in my action. I wonder if it's better to use GITHUB_WORKSPACE here:

clang-tidy-pr-comments/entrypoint.sh

Line 17 in 5aec7f4

recreated_runner_dir="/home/runner/work/$repository_name"

instead of an absolute path. (The solution provided by @kenji-miyake does work, but I think it's sort of hacking...) What do you think?

[oleg-derevenetz]

Hi @caic99 I'm not sure that this will really work because the directory that GITHUB_WORKSPACE should point to does not exist in the action's container (this directory is being created in the entrypoint.sh by hand), so I'm not sure that this variable is available in this container at all. However, you can try it yourself and offer PR if it really works.

[oleg-derevenetz]

@caic99 Yeah, just like I suspected, it seems that the GITHUB_WORKSPACE variable points to the different directory inside a custom action container:

https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#file-systems

[caic99]

@oleg-derevenetz OK, I'll provide details in a new issue, and try to fix it.