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
x/review/git-codereview: hooks does not re-install hooks #16777
Comments
I don't think overwriting the hooks is a good idea either; the user might have their own hooks that they're not expecting to be overwritten. Perhaps a "-f" to overwrite hooks, and a warning if -f is not supplied? |
@josharian for decision |
We shouldn't overwrite existing hooks. That much seems clear. I'd rather not add more flags and I'd rather not have the tool delete anything; this is rarely used functionality, I suspect, which means the opportunity for latent bugs and data loss is higher. Perhaps when there are existing hooks, we could print a message like:
Thoughts? |
SGTM. |
Change https://golang.org/cl/184417 mentions this issue: |
Change https://golang.org/cl/377034 mentions this issue: |
Hi, It looks like even the last CL was not merged. How can I see what is left to do? |
@BerndCzech Thanks for looking to contribute to Go. There is indeed an existing CL here, and a next step before it can be submitted is for it to be reviewed. If you're interested, please see https://go.dev/doc/contribute#reviews and https://go.dev/doc/contribute#download in particular. It would help if you can test out the code change to make sure it works as expected, and leave comments if you find opportunities to improve it. I plan to also leave some review comments on CL 377034 soon. Thanks. |
ty. I could make it run on my machine - I was not able to add new Review comments but I hope I could add some value to yours. If the former contributor does not respond anymore would I create a new CL or takeover his? |
Display warning message when a hook is already installed and is different from the one installed by git-codereview. Improves upon CL 184417. Fixes golang/go#16777 Change-Id: I7579a3e86572e8b74f92317973e7cc7094b3942d
Display warning message when a hook is already installed and is different from the one installed by git-codereview. Improves upon CL 184417. Fixes golang/go#16777 Change-Id: I7579a3e86572e8b74f92317973e7cc7094b3942d
When calling
git-codereview hooks
, it shouldn't skip over hook files that already exist.My GOROOT had the default Gerrit hooks instead of git-codereview's hooks.
installHook probably needs a flag to skip this block of code:
Or, we could use file size as a heuristic to re-install the hooks, since we're already statting the file.
The text was updated successfully, but these errors were encountered: