-
Notifications
You must be signed in to change notification settings - Fork 1.6k
ci: add local pre-commit DCO hook and setup command #9094
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| #!/usr/bin/env bash | ||
| # DCO sign-off check for the pre-commit commit-msg stage. | ||
| # | ||
| # Mirrors the GitHub DCO app requirement: every commit must carry a | ||
| # "Signed-off-by:" line identifying the author. | ||
| # | ||
| # Usage: check-dco.sh <commit-message-file> | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| msg_file="${1:-}" | ||
|
|
||
| if [[ -z "${msg_file}" || ! -f "${msg_file}" ]]; then | ||
| echo "DCO check: no commit message file supplied." >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| if grep -qE '^Signed-off-by: .+ <[^@ ]+@[^@ ]+>$' "${msg_file}"; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| cat >&2 <<'EOF' | ||
| DCO check failed: commit message is missing a "Signed-off-by:" line. | ||
|
|
||
| Add a sign-off using one of: | ||
| git commit -s # sign as you create the commit | ||
| git commit --amend -s # sign the most recent commit | ||
|
|
||
| The line must identify the commit author, for example: | ||
| Signed-off-by: Your Name <you@example.com> | ||
| EOF | ||
|
|
||
| exit 1 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,7 +51,19 @@ To collaborate efficiently, please read through this section and follow them. | |
|
|
||
| #### Checking the coding style | ||
|
|
||
| Coding style is checked and enforced by black, isort, and ruff. | ||
| Coding style is checked and enforced by black, isort, and ruff, and every commit must carry a DCO sign-off. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this content should go below with the section discussing DCO and signoff, users won't know what DCO is at this point in the document. We should also mention somewhere about setting your git global variables to use in the signoff. With [global]
[user]
name = Eric Kerfoot
email = 17726042+ericspod@users.noreply.github.comThe no-reply email address Github assigns to each user can be seen at https://github.com/settings/emails. As I use here, it's more private to use this address rather than an actual email, and it's associated directly with your Github username. We should also mention that VSCode can be set to automatically sign off on every commit. |
||
| To catch formatting and DCO failures before they reach CI, install the git pre-commit hooks once per checkout: | ||
|
|
||
| ```bash | ||
| # install the git hooks: black, isort, ruff, and the DCO sign-off check | ||
| pre-commit install | ||
|
|
||
| # or, via the test runner: | ||
| ./runtests.sh --setup | ||
| ``` | ||
|
|
||
| These hooks run automatically on every `git commit`: `black`, `isort`, and `ruff` reformat the staged files, and the `commit-msg` hook blocks a commit that is missing a `Signed-off-by` line. | ||
|
|
||
| Before submitting a pull request, we recommend that all linting should pass, by running the following command locally: | ||
|
|
||
| ```bash | ||
|
|
@@ -247,6 +259,8 @@ Git has a `-s` (or `--signoff`) command-line option to append this automatically | |
| git commit -s -m 'a new commit' | ||
| ``` | ||
|
|
||
| If the git pre-commit hooks are installed (`pre-commit install` or `./runtests.sh --setup`), the local `commit-msg` hook blocks any commit that is missing this line, so the DCO check fails locally rather than in CI. | ||
|
|
||
| The commit message will be: | ||
|
|
||
| ``` | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -53,6 +53,7 @@ doPyreflyFormat=false | |||||||||||||
| doCleanup=false | ||||||||||||||
| doDistTests=false | ||||||||||||||
| doPrecommit=false | ||||||||||||||
| doSetup=false | ||||||||||||||
| testTimeout=0 | ||||||||||||||
|
|
||||||||||||||
| NUM_PARALLEL=1 | ||||||||||||||
|
|
@@ -61,7 +62,7 @@ PY_EXE=${MONAI_PY_EXE:-$(which python)} | |||||||||||||
|
|
||||||||||||||
| function print_usage { | ||||||||||||||
| echo "runtests.sh [--codeformat] [--autofix] [--black] [--isort] [--pylint] [--ruff]" | ||||||||||||||
| echo " [--clangformat] [--precommit] [--pytype] [-j number] [--pyrefly]" | ||||||||||||||
| echo " [--clangformat] [--precommit] [--pytype] [-j number] [--pyrefly] [--setup]" | ||||||||||||||
| echo " [--unittests] [--disttests] [--coverage] [--quick] [--min] [--net] [--build] [--list_tests]" | ||||||||||||||
| echo " [--dryrun] [--copyright] [--clean] [--help] [--version] [--path] [--formatfix]" | ||||||||||||||
| echo "" | ||||||||||||||
|
|
@@ -103,6 +104,7 @@ function print_usage { | |||||||||||||
| echo "" | ||||||||||||||
| echo "Misc. options:" | ||||||||||||||
| echo " --dryrun : display the commands to the screen without running" | ||||||||||||||
| echo " --setup : install git pre-commit hooks (black, isort, ruff, DCO sign-off)" | ||||||||||||||
| echo " --copyright : check whether every source code has a copyright header" | ||||||||||||||
| echo " -f, --codeformat : shorthand to run all code style and static analysis tests" | ||||||||||||||
| echo " -c, --clean : clean temporary files from tests and exit" | ||||||||||||||
|
|
@@ -320,6 +322,9 @@ do | |||||||||||||
| --precommit) | ||||||||||||||
| doPrecommit=true | ||||||||||||||
| ;; | ||||||||||||||
| --setup) | ||||||||||||||
| doSetup=true | ||||||||||||||
| ;; | ||||||||||||||
| --pytype) | ||||||||||||||
| echo "${yellow}WARNING: --pytype is deprecated and may be removed in a future release.${noColor}" | ||||||||||||||
| doPytypeFormat=true | ||||||||||||||
|
|
@@ -429,6 +434,21 @@ then | |||||||||||||
| echo "${green}done!${noColor}" | ||||||||||||||
| fi | ||||||||||||||
|
|
||||||||||||||
| if [ $doSetup = true ] | ||||||||||||||
| then | ||||||||||||||
| echo "${separator}${blue}setup${noColor}" | ||||||||||||||
|
|
||||||||||||||
| # ensure pre-commit is available | ||||||||||||||
| if ! is_pip_installed pre_commit | ||||||||||||||
| then | ||||||||||||||
| install_deps | ||||||||||||||
| fi | ||||||||||||||
|
|
||||||||||||||
| ${cmdPrefix}"${PY_EXE}" -m pre_commit install | ||||||||||||||
|
|
||||||||||||||
| echo "${green}done! git hooks installed (black, isort, ruff, DCO sign-off).${noColor}" | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Do not report hooks as installed in dry-run mode. With Proposed fix- echo "${green}done! git hooks installed (black, isort, ruff, DCO sign-off).${noColor}"
+ if [[ -z "$cmdPrefix" ]]; then
+ echo "${green}done! git hooks installed (black, isort, ruff, DCO sign-off).${noColor}"
+ else
+ echo "dry-run: git hooks would be installed (black, isort, ruff, DCO sign-off)."
+ fi📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
| fi | ||||||||||||||
|
|
||||||||||||||
| # unconditionally report on the state of monai | ||||||||||||||
| print_version | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: Project-MONAI/MONAI
Length of output: 4486
🤖 get_repo_knowledge executed:
get_repo_knowledge Project-MONAI/MONAI /tmp/coderabbit-repo-knowledge/project-monai-monai-43c44fdeLength of output: 351
🏁 Script executed:
Repository: Project-MONAI/MONAI
Length of output: 423
🏁 Script executed:
Repository: Project-MONAI/MONAI
Length of output: 353
🏁 Script executed:
Repository: Project-MONAI/MONAI
Length of output: 1035
Accept CRLF commit-message files.
When the message file uses CRLF, line 18 does not match the trailing
\r, so thecommit-msghook rejects a valid sign-off. Allow an optional\rbefore$and add a CRLF test.🤖 Prompt for AI Agents