lint: Convert lint-git-commit-check.sh to Python #24853

pull Kvaciral wants to merge 1 commits into bitcoin:master from Kvaciral:lint-git-commit-check-port changing 2 files +63 −48

Kvaciral commented at 12:09 PM on April 14, 2022: contributor

A port of /test/lint/lint-git-commit-check.sh to a Python-script as part of the request of #24783 . Checked for output-consistency.
DrahtBot added the label Tests on Apr 14, 2022
Kvaciral force-pushed on Apr 14, 2022

laanwj commented at 2:46 PM on April 14, 2022: member

Tested ACK 2e6cfc73007847b7c51de8f586cb67bd605200ed re-ACK f27fcd9bf4d667f78055d4b3ee6f426e4c5ffe16

$ git commit --allow-empty -m $'invalid\ncommit\nmessage'
[pull/24853/local-merge 8e2cddbb3a6163d47c728bdbb93bd57867bd180c] invalid commit message
$ test/lint/lint-git-commit-check.py
The subject line of commit hash 8e2cddbb3a6163d47c728bdbb93bd57867bd180c is followed by a non-empty line. Subject lines should always be followed by a blank line.
$ git reset --hard HEAD~1
$ test/lint/lint-git-commit-check.py

Kvaciral force-pushed on Apr 14, 2022
lint: Convert lint-git-commit-check.sh to Python f27fcd9bf4
in test/lint/lint-git-commit-check.py:27 in 2e6cfc7300 outdated
```
  22 | +            the body or no body at all.
  23 | +        """,
  24 | +        epilog="""
  25 | +            You can manually set the commit-range with the COMMIT_RANGE
  26 | +            environment variable. Defaults to current merge base when
  27 | +            neither prev-commits nor the environment variable is set.
```
anibilthare commented at 10:04 AM on April 16, 2022:

I think it will be helpful to the user if we add an usage example for COMMIT_RANGE environment variable here, just how the help manual for test/lint/lint-git-commit-check.sh does it, something like: COMMIT_RANGE='b69fd5e...2e6cfc7' test/lint/lint-git-commit-check.py

Kvaciral commented at 1:41 PM on April 16, 2022:

Yes, you're right!
Kvaciral force-pushed on Apr 16, 2022
in test/lint/lint-git-commit-check.py:50 in f27fcd9bf4
```
  45 | +            merge_base = check_output(["git", "merge-base", "HEAD", "master"], universal_newlines=True, encoding="utf8").rstrip("\n")
  46 | +            commit_range = merge_base + "..HEAD"
  47 | +    else:
  48 | +        commit_range = os.getenv("COMMIT_RANGE")
  49 | +
  50 | +    commit_hashes = check_output(["git", "log", commit_range, "--format=%H"], universal_newlines=True, encoding="utf8").splitlines()
```
anibilthare commented at 3:59 PM on April 17, 2022:

Having an exception placed here would be nice? I was testing the code and figured out that it would crash if we set COMMIT_RANGE to something very random, like 5 . If the code generates an exception we can convey something like Please check the commit range to the user?

Kvaciral commented at 12:56 PM on April 18, 2022:

I prefer to not do that here, the current output (the git log-stuff and the subprocess-error) is clear enough. Besides, commit_range is usually only used by CI .. and furthermore, the lint-scripts are manually only used by devs and not (technical illiterate) end-users..
laanwj merged this on Apr 18, 2022
laanwj closed this on Apr 18, 2022
sidhujag referenced this in commit d100a1c133 on Apr 19, 2022
bitcoin deleted a comment on Apr 27, 2022
bitcoin locked this on Oct 11, 2023

Contributors

Labels

Linked (view graph)

#24783 Port lint scripts to Python