search

ZedThree / clang-tidy-review

Create a pull request review based on clang-tidy warnings
MIT License
90 stars 45 forks source link

pull_request merge not accounted for #10

Closed vadi2 closed 3 years ago

vadi2 commented 3 years ago

When Github is running a check for a PR, it merges it into the incoming branch first. The action however calculates the line numbers diff on the pre-merge file.

This means that if the file in question was changed in development by some other code, the action won't work. If it hasn't been changed in development yet, the action will work.

ZedThree commented 3 years ago

Thanks for looking into this @vadi2 ! This might be a little bit of a problem, because we need to use the Github diff pre-merge to get the line numbers in the PR to post comments on. It works like this:

  1. Download the Github PR diff
  2. Create a lookup table of source code line numbers to PR diff line number
  3. Run clang-tidy on just the changed lines
  4. For each warning, convert source code line number to PR diff line number

With the Github merge commit, it's not obvious how to construct the lookup table for step 2. It's at least going to be a bit more complicated.

It looks like there's an option for checking out the PR HEAD instead of the merge commit: https://github.com/actions/checkout#checkout-pull-request-head-commit-instead-of-merge-commit That might be the easiest thing to do -- I'll investigate and put that in the README if it turns out to be the best course of action

vadi2 commented 3 years ago

I think that would work :+1: