Skip to content
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

git: skip running pre-push hooks when subprocessing to match git2 behaviour #5612

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

bsdinis
Copy link
Contributor

@bsdinis bsdinis commented Feb 6, 2025

This fixes #5610

lib/src/git_subprocess.rs Outdated Show resolved Hide resolved
@bsdinis bsdinis force-pushed the bsdinis/mzqymxuzlnyz branch from 7579c59 to c098b8a Compare February 6, 2025 21:36
@bsdinis bsdinis force-pushed the bsdinis/mzqymxuzlnyz branch from c098b8a to 805c123 Compare February 6, 2025 21:37
Copy link
Member

@martinvonz martinvonz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but let's give @arxanas a chance to object before merging.

Copy link
Contributor

@arxanas arxanas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, for this PR, I just wanted to update the comment to specify that it's an intermediate state of indecision rather than the ultimate state, rather than make any specific decision about pre-push hooks right now. It looks good to me now that the comment includes references to the relevant issues.

@bsdinis bsdinis added this pull request to the merge queue Feb 6, 2025
@arxanas
Copy link
Contributor

arxanas commented Feb 6, 2025

Let's update the comment to explain whether or not this is the end goal.

Oops, sorry about this. I see now that I didn't write what I wanted to write. Originally, I wrote something like "let's update the comment to say that this is not the end goal", but then I realized we technically did not have agreement on whether it was or wasn't the end goal, so I tried to adjust the wording, and did so wrongly.

Merged via the queue into main with commit 0c446c9 Feb 6, 2025
38 checks passed
@bsdinis bsdinis deleted the bsdinis/mzqymxuzlnyz branch February 6, 2025 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Git operations with git.subprocess=true fail if hook prints unexpected text
4 participants