Codecov Report

Attention: Patch coverage is 98.72611% with 2 lines in your changes missing coverage. Please review.

Project coverage is 99.88%. Comparing base (854c662) to head (3bb8f33).
Report is 5 commits behind head on main.

@@             Coverage Diff             @@
##              main     #117      +/-   ##
===========================================
- Coverage   100.00%   99.88%   -0.12%     
===========================================
  Files           23       24       +1     
  Lines         1732     1798      +66     
===========================================
+ Hits          1732     1796      +64     
- Misses           0        2       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Everything seems correct in the test PR.

I've got a new idea to merge the suggestions from both tools... Instead of each tool posting a separate review comment, I might be able to merge the comments from both tools if their line numbers match up. This idea is different from merging the patches from each tool.

I've got a new idea to merge the suggestions from both tools...

Good idea! Just to confirm, if the user only uses clang-format or clang-tidy, they can still work.

if the user only uses clang-format or clang-tidy, they can still work.

Yes

Just to illustrate these changes:

data structures (inheritence)

graph TD
  FileObj -- contains --> TidyAdvice -- inherits from --> PatchMixin
  FileObj -- contains --> FormatAdvice -- inherits from --> PatchMixin

review comment dataflow

Changes are captured

flowchart TD
  cachePatch["save suggested changes
  to PatchMixin.patched"]
  capTidy["`run_clang_tidy()`"] --> cachePatch
  capFormat["`run_clang_format()`"] --> cachePatch

Review comments are created

Starting from GithubApiClient.post_feedback() which will appropriately call GithubApiClient.post_review().

%%{init: {"flowchart": {"htmlLabels": false}} }%%
flowchart TD
  postReviews["post_reviews()"] --> getComments["create_review_comments()"]
  subgraph for each file in files
    getFmtSuggestion["FormatAdvice
    .get_suggestions_
    from_patch()."]
    getTidySuggestion["TidyAdvice
    .get_suggestions_
    from_patch()."]
    getFmtSuggestion o-- Here we merge suggestions with existing comments --o getTidySuggestion
  end
  serialize["serialize comments
  into JSON payload
  (review summary created here)"] --> post["post new review"]

  getComments -- if format-review is true --> getFmtSuggestion --> serialize
  getComments -- if tidy-review is true --> getTidySuggestion --> serialize

I've got a new idea to merge the suggestions from both tools...

Success! 🎉

There's still some edge cases where the suggestion comments cannot be merged, but I'm happier with the new results (compared to old/stable results).

There's also a couple lines that get missed in coverage. This is because the test conditions do not include a scenario where a clang-tidy diagnostic comment (with no suggested fix -- see below) cannot be merged into existing review comments.

I'm ok with the loss of coverage in this situation (for now).

The diagram and the final picture look amazing!

I realized I keep using dense/complex sentences (with double negatives), and the code is not super simple. That's why I created the graphs and picture.

I feel v1.10.1 might be a minor release insead of patch. It's amazing, no matter what. 👍