On a practical note, do we understand what went wrong and how to prevent it in the future? Do we need to improve cherry-picking documentation or tooling for committers?
- Michael On Tue, Nov 29, 2022 at 3:41 AM PengHui Li <peng...@apache.org> wrote: > > It's not only two commits > There are more than 10 commits with meaningless commit messages. > Please check the commit logs > https://github.com/apache/pulsar/tree/branch-2.9-backup > > > Prettifying the git commit history isn't a proper reason to do force > pushes > > I don't think it should be `Prettifying` here. You are not able to grep the > commit logs > with the keyword. It's an error commit message, not an unpretty message. > It will be inconvenient for users to check whether the fix is backported to > branch-2.9. > If we revert all of them, we will see 20~30 unreasonable commits. > > And the commit only happened these days, and we don't have a release for it. > I think it makes sense to correct it before we release it. Otherwise, the > error commits > message will persist for a long time. > > Additionally, we have a backup for branch-2.9. > > This does not mean that I support force push to branch at any time. > For some occasional cases, I think we can talk case by case. > > Thanks, > Penghui > > On Tue, Nov 29, 2022 at 3:21 PM Lari Hotari <lhot...@apache.org> wrote: > > > +1 to what Michael said. > > > > If an invalid commit gets pushed into one of the public branches, it will > > simply need to be reverted. Prettifying the git commit history isn't a > > proper reason to do force pushes. There could be special exceptions, such > > as when someone accidentally pushes confidential information in the git > > repository. That wasn't the case this time. I hope that the next time that > > there's a need for force pushes that priv...@pulsar.apache.org is > > consulted first. > > > > -Lari > > > > On 2022/11/28 22:51:18 Michael Marshall wrote: > > > I do not think we should have done a force push to rewrite two commit > > > messages. The problematic messages had commit hashes in their names, > > > so it was easy enough to figure out what went wrong and how to find > > > the original commit. If it was really important to fix the messages, I > > > think we should have reverted the problematic commits and re-done the > > > cherry pick with the desired commit messages. > > > > > > I also do not think we should allow force pushing to release branches > > > or to master branch in the future. In my opinion, the git history > > > should be immutable. > > > > > > Thanks, > > > Michael > > > > > > > > > On Fri, Nov 25, 2022 at 11:39 PM 丛搏 <bog...@apache.org> wrote: > > > > > > > > +3 binding(Penghui Li, JiWei guo, Hang Chen) +2 nonbing(Zike Yang, > > > > Yubiao Feng), so I will close this discussion and merge > > > > https://github.com/apache/pulsar/pull/18623. thanks for your votes. > > > > > > > > Thanks, > > > > bo > > > > > > > > 丛搏 <congbobo...@gmail.com> 于2022年11月25日周五 18:55写道： > > > > > > > > > > > > > > > > > One thing I'd like to make clear: this is a temporary solution, > > and we'll re-enable branch protection after the actions are performed (or > > relax to 2.9.4 released). > > > > > yes, after this pr: https://github.com/apache/pulsar/pull/18623 > > merged > > > > > and reset branch-2.9 to commit 5ed247de3a, I will push a new PR to > > the > > > > > master branch to open the branch-2.9 force push > > > > > > > > > > > > If it's the case, please file an issue and assign yourself to > > revert on time :) > > > > > if no one is against it in 24 hours, this discussion will be approved > > > > > then merge https://github.com/apache/pulsar/pull/18623 then reset > > > > > branch-2.9 to commit 5ed247de3a > > > > >
