Preparing for v2.9.0 PR review feature release

[shenxianpeng]

To release v2.9.0 PR review and other improvements need to complete the following tasks

• Release cpp-linter/cpp-linter v1.7.0
• Update README.md file. refer to Preparing for v2.9.0 PR review feature release #182 (comment)
• Add a screenshot of the new feature PR review to show how it works
• Include State what python version is used in README #187

[2bndy5]

from test PR 1

from test PR 2

---

There are other caveats to note about this feature

1. The "GitHub Actions" bot may need to be allowed to approve PRs. This can be done in the repo settings (or in org settings if applicable). By default, the bot cannot approve PR changes, only request more changes. This will show as a logged warning if the given token isn't configured with the proper permissions.
2. The feature is auto-disabled for
   • closed PRs
   • PRs marked as "draft"
   • push events
3. Clang-tidy and clang-format suggestions are shown in 1 PR review.
   • Users are encouraged to choose either tidy-review or format-review. Enabling both will likely show duplicate or similar suggestions. Remember, clang-tidy can be configured to use the same style that clang-format accepts. There is no current implementation to combine suggestions from both tools (clang-tidy kind of does that anyway).
   • Each generated review is specific to the commit that triggered the CI.
   • Outdated reviews are dismissed but not marked as resolved. Also, the outdated review's summary comment is not automatically hidden. To reduce PR thread noise, users interaction is required. GitHub REST API does not provide a way to hide comments or mark review suggestions as resolved.
4. If any suggestions did not fit within the PR diff, then the review's summary comment will indicate how many suggestions were left out. The full patch of suggestions is always included as a collapsed code block in the review summary comment. This isn't a problem we can fix. GitHub won't allow review comments/suggestions to target lines that are not shown in the PR diff.
   • Users are encouraged to set lines-changed-only to true. This will help us keep the suggestions limited to lines that are shown within the PR diff. However, there are still some cases where clang-format or clang-tidy will apply fixes to lines that are not within the diff. This can't be avoided because the --line-filter passed to the clang-tidy (and --lines passed to clang-format) only applies to analysis, not fixes.
   • Not every diagnostic from clang-tidy can be automatically fixed. Some diagnostics require user interaction/decision to properly address.
   • Some fixes provided might depend on what compiler is used. I have made it so clang-tidy takes advantage of any fixes provided by the compiler. Compilation errors may still prevent clang-tidy from reporting all concerns.

[shenxianpeng]

I created this ticket only to remember to do it. I'm sorry to let you spend time on this. I would not publish this feature very soon until I completed my other todos and then learn this feature.

I also apologize for bothering you this past weekend as I hadn't noticed your email before. All the best.

[2bndy5]

Its no problem. These short comments are a welcome escape from reality, for now.

[2bndy5]

As I typed that list of caveats, I thought that it might be desirable to have only the PR review's summary comment (with no suggestions shown in the diff). To do that, I would probably support a env var named CPP_LINTER_PR_REVIEW_NO_SUGGESTION. That should help reduce PR thread pollution.

[2bndy5]

picture of clang-tidy review comment(s):

[2bndy5]

picture of clang-format review summary comment:

I didn't include any suggested changes here because they are a bit misleading (although accurate and expected). A more accurate preview of the PR review feature using clang-format would be something like:

Not this (from same review)

[2bndy5]

Picture of approved review from both tools: