-
-
Notifications
You must be signed in to change notification settings - Fork 638
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
Replaced pre-commit githook with pre-push #20784
Conversation
@@ -61,32 +61,30 @@ To free up space, run `rm -rf src/rust/engine/target`. | |||
Warning: this will cause Rust to recompile everything. | |||
::: | |||
|
|||
## Step 3: (Optional) Set up a Git hook | |||
## Step 3: Set up a pre-push Git Hook |
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.
Removing "Optional"
Seems weird to write that in the development docs. For the people who develop often, they're used to the workflow and can do as they please anyways.
For those who are new to committing to Pants, this feels like a simple thing that would be nice to save some CI loops.
|
||
To install this, run: | ||
|
||
```bash | ||
$ build-support/bin/setup.sh | ||
``` | ||
|
||
If you commit frequently as part of your workflow you may find it annoying to have these run every time, so installing this hook is not required. |
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.
Removing "Optional".
The frequency of run is now substantially lower, since it's on push instead of commit.
@kaos Just a heads up that I added a function which will look for broken symlinks in the githooks, and ask the user if they want to delete it. Just a bit of house keeping. |
The fact that we wrote:
explains why I'm making this change :)
Closes #20541