ci: Skip COMMIT_RANGE if no CIRRUS_PR #26588

1. 
   maflcko commented at 10:13 am on November 28, 2022: member

   It doesn’t make sense to run this for non-PRs, because:

   • There are known whitespace “violations” in previous commits, so the lint may fail
   • Once the changes are merged, it is too late to fix them up (force pushes are illegal)
   • It isn’t possible to determine which commits to run on if there is no reference branch (target branch of the pull request)

   Moreover, the test fails on non-master:

   • https://github.com/bitcoin/bitcoin/runs/8664441400

   Fix all issues by skipping it.

2. lint: Skip COMMIT_RANGE if no CIRRUS_PR fad1c55301
3. 
   DrahtBot commented at 10:13 am on November 28, 2022: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	hebasto

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

4. 
   hebasto commented at 10:24 am on November 28, 2022: member

   Concept ACK. Thank you for taking this up!
5. in ci/lint/06_script.sh:14 in fad1c55301

    8@@ -9,7 +9,12 @@ export LC_ALL=C
    9 GIT_HEAD=$(git rev-parse HEAD)
   10 if [ -n "$CIRRUS_PR" ]; then
   11   COMMIT_RANGE="${CIRRUS_BASE_SHA}..$GIT_HEAD"
   12+  echo
   13+  git log --no-merges --oneline "$COMMIT_RANGE"
   14+  echo
   

   ---

   ---

   

   hebasto commented at 11:19 am on November 28, 2022:

   I’m not sure if moving git log --no-merges --oneline "$COMMIT_RANGE" up is an improvement. At least for me, it is very convenient to look at the list of commits at the end of the log.

   ---

   

   maflcko commented at 11:54 am on November 28, 2022:

   Is there even any value in keeping it. Now that it only runs on PRs, it should be only be needed for rare debugging of CI infrastructure bugs? In the normal case it will just be equal to the list of commits that GitHub displays, no?

   ---

   

   hebasto commented at 12:07 pm on November 28, 2022:

   In the normal case it will just be equal to the list of commits that GitHub displays, no?

   Exactly. That is why I check it out to ensure that it is normal :)

   Anyway, I’m OK with the current state of this PR.

6. hebasto approved
7. 
   hebasto commented at 11:22 am on November 28, 2022: member

   ACK fad1c55301b9f2d091d3b0d8a75ff522ce8dae5a, also tested in my personal Cirrus account.
8. 
   hebasto commented at 11:23 am on November 28, 2022: member

   Should this PR be backported to release branches?
9. maflcko renamed this:
   lint: Skip COMMIT_RANGE if no CIRRUS_PR
   ci: Skip COMMIT_RANGE if no CIRRUS_PR
   on Nov 28, 2022
10. DrahtBot added the label Tests on Nov 28, 2022
11. maflcko merged this on Nov 28, 2022
12. maflcko closed this on Nov 28, 2022

    ---

13. maflcko referenced this in commit f668a3a859 on Nov 28, 2022
14. maflcko deleted the branch on Nov 28, 2022
15. sidhujag referenced this in commit 83cdd00c76 on Dec 1, 2022
16. bitcoin locked this on Nov 28, 2023

Contributors
maflcko DrahtBot hebasto

Labels
Tests