-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add githook script to alert on changes that require re-installs #26
base: master
Are you sure you want to change the base?
Conversation
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.
PR Summary
This PR introduces a new Git hook to check for dependency changes when switching branches or merging, enhancing developer awareness of potential project setup modifications.
- Added local pre-commit hook in
.pre-commit-config.yaml
to runscripts/githook_deps_check.py
- Implemented new
init-githooks
target inMakefile
for installing all Git hooks - Created
scripts/githook_deps_check.py
to perform dependency change checks and alert users - Script uses environment variables and 'rich' library for colorful console output
- TODO comments in the script indicate plans for future enhancements (e.g., warn on rebase, pull)
3 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings
name: Check for dependency changes | ||
entry: python -m scripts.githook_deps_check | ||
language: system | ||
always_run: true |
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.
style: Consider setting always_run: false
to improve performance, as the script likely doesn't need to run on every commit.
init-precommit: ## install the pre-commit hook into your local git repository | ||
($(VENV_RUN); pre-commit install) |
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.
syntax: typo in comment: 'te' should be 'the'
if os.environ.get(DISABLE_CHECKS_ENV_VAR, "0") == "1": | ||
exit(0) |
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.
style: Add a log message when checks are disabled to inform the user.
if from_commit and to_commit: | ||
r = subprocess.run( | ||
["git", "diff", "--name-only", from_commit, to_commit], capture_output=True | ||
) |
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.
style: Use 'git diff --name-only' with '--no-renames' flag to avoid issues with renamed files.
for file in files_to_watch: | ||
if file in files: | ||
c.print( | ||
f"[red]ATTENTION![/red] Found a change in file [red]{file}[/red] during the checkout." | ||
) | ||
c.print( | ||
f"\t>> show changes: [bold]git diff {from_commit[:8]}:{file} {to_commit[:8]}:{file}[/bold]\n" | ||
) |
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.
style: Consider using a set for faster lookup of changed files.
Motivation
Uses pre-commit to register a local python script that will be executed after running git checkout, i.e. when switching branches. It contains a list of files and if the diff between the two commits includes a change in these files, it will output a warning.
How to use:
Run make init-githooks to set up the hooks for your current repository.
You can trigger the behavior by switching to any branch with some change in any of the files that are listed in the script.
Some things that I think should still be included before an initial merge:
References:
https://git-scm.com/docs/githooks
https://github.com/pre-commit/pre-commit
Changes