golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.8k stars 17.64k forks source link

x/review/git-codereview: give an error if the Change-Id: line is deleted #10386

Open minux opened 9 years ago

minux commented 9 years ago

I think this is the reason why some CLs have more than one Gerrit CL.

Given that using the same change to open a fresh new gerrit CL is rarely needed, I think git-codereview should error out if the user deletes the Change-Id: line (with git change).

And we can add a -f option to override the error (I'm also fine with let the user git commit --amend to manually remove the Change-Id.)

Of course, git-codereview probably can't detect the case where the user has used git commit --amend to remove the Change-Id, but our contribution guideline uses git change, so the proposed check should catch most problems.

josharian commented 9 years ago

Good idea. How would we propagate the necessary state from git change to the hook? We don't have much context in the hook at this point. In particular, I don't think we have enough context to distinguish this from a commit message coming from a new commit in the same branch (i.e. running regular git commit).

ysmolski commented 5 years ago

IMHO, this cannot be done. I checked what @josharian said. The problem is reduced to: how do we detect that Change_id was removed at all? There is not enough info.