Justin Tobler <jltobler@xxxxxxxxx> writes: > On 24/07/08 11:23AM, Karthik Nayak wrote: >> We don't run style checks on our CI, even though we have a >> '.clang-format' setup in the repository. Let's add one, the job will >> validate only against the new commits added and will only run on merge >> requests. Since we're introducing it for the first time, let's allow >> this job to fail, so we can validate if this is useful and eventually >> enforce it. > > [snip] > >> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml >> index 37b991e080..65fd261e5e 100644 >> --- a/.gitlab-ci.yml >> +++ b/.gitlab-ci.yml >> @@ -123,6 +123,18 @@ check-whitespace: >> rules: >> - if: $CI_PIPELINE_SOURCE == 'merge_request_event' >> >> +check-style: >> + image: ubuntu:latest >> + allow_failure: true >> + variables: >> + CC: clang >> + before_script: >> + - ./ci/install-dependencies.sh >> + script: >> + - ./ci/run-style-check.sh "$CI_MERGE_REQUEST_DIFF_BASE_SHA" > > One downside to using $CI_MERGE_REQUEST_DIFF_BASE_SHA is that for GitLab > merge pipeines, commits from the merge that are not part of the MR > changes are also included. This could lead to somewhat confusing > failures. > I'm not sure I follow. > Example failure occuring on this patch series: > https://gitlab.com/gitlab-org/git/-/jobs/7284442220 > If you notice this job, it points to the commit: 1c6551488, and the parent commit of that commit is: 614dff2011. The parent commit [1] is a test commit I added to check the failures. So isn't this working as expected? > It might be best to use $CI_MERGE_REQUEST_TARGET_BRANCH_SHA instead. > I actually started with $CI_MERGE_REQUEST_TARGET_BRANCH_SHA, it didn't work, because the value was undefined. See: https://gitlab.com/gitlab-org/git/-/jobs/7283724903 This is why I also decided to fix and change the whitespace check. >> + rules: >> + - if: $CI_PIPELINE_SOURCE == 'merge_request_event' >> + [1]: https://gitlab.com/gitlab-org/git/-/commit/614dff20119aa325661424a9fcef552e242a95d9
Attachment:
signature.asc
Description: PGP signature
