Justin Tobler [off-list ref] writes:

On 24/07/08 11:23AM, Karthik Nayak wrote:

quoted

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]

quoted

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.

quoted

+  rules:
+    - if: $CI_PIPELINE_SOURCE == 'merge_request_event'
+

[1]: https://gitlab.com/gitlab-org/git/-/commit/614dff20119aa325661424a9fcef552e242a95d9