AaronBallman wrote: > > > As I understood the author, more time was needed to prepare the tests and > > > the fix. I just tried to bring the tip of the tree to a good state soon > > > (our internal release process was blocked on this) without putting any > > > pressure on the author. > > > > > > Typically when the author is engaging fairly quickly and only needs a short > > time (he had already identified the problem and only had process-related > > concerns), it is preferred to be understanding/polite and give them time to > > work on it. > > First of all, let me quote the developer policy: ["Remember, it is normal and > healthy to have patches reverted. Having a patch reverted does not > necessarily mean you did anything > wrong."](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy). > Now, I totally understand the desire to "capture the progress" as well as > frustration one might have seeing their work reverted. However, a revert > shouldn't be perceived as an impolite or unfriendly action. All contributions > are welcome and appreciated. Reverting a commit is merely a way to deal with > problems one at a time without blocking others or racing against the clock.
The developer policy also says: "Reverts should be reasonably timely. A change submitted two hours ago can be reverted without prior discussion. A change submitted two years ago should not be. Where exactly the transition point is is hard to say, but it’s probably in the handful of days in tree territory. If you are unsure, we encourage you to reply to the commit thread, give the author a bit to respond, and then proceed with the revert if the author doesn’t seem to be actively responding." "Reasonably timely" also means that authors who are engaged on a fix are expected to have the opportunity to land that fix. Reverts can be disruptive and they can clutter the revision history, so *if* we can avoid them, it's better to avoid them. However, we don't want to avoid reverts when they help keep the tree stable; not reverting can be plenty disruptive, too! There's a happy medium somewhere between "revert as quickly as possible at the first sign of trouble" and "let authors have infinite time to fix issues". I think it's better that we err on the side of reverting quickly, and I don't think anyone has done anything wrong here. But I think we have some downstream projects which are quicker to revert than is comfortable and so some code owners are pushing back (gently) on reverts that we think may have jumped the gun a bit. For example, I know I've personally had several instances where I've had things reverted out from under me as I'm trying to land the fix and that's caused significantly more work for me. This review seems to have had a similar outcome: the community was told about the issue, the patch author immediately responded, and the patch revert happened regardless and more effort is required from everyone as a result. Again, I still think the revert is defensible. But I also think the push back on the speed at which the revert happened is defensible as well. This is mostly just a call to coordinate a bit better. I would suggest that if the patch author is engaged on the thread, it would make sense to tell them the schedule on which you plan to revert so they have an opportunity to tell you if that's going to be disruptive. https://github.com/llvm/llvm-project/pull/77768 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
