[Wikitech-l] When to automerge (Re: Revoking +2 (Re: who can merge into core/master?))
On 15/02/13 01:38, Brad Jorsch wrote: I'd propose one more: * Someone else gives +2, but Jenkins rejects it because it needs a rebase that is not quite trivial enough for it to do it automatically. For example, something in RELEASE-NOTES-1.21. Seems a better example. I'm not convinced that backporting should be automatically merged, though. Even if the code at REL-old is the same as master (ie. the backport doesn't needs any code change), approving something from master is different than agreeing that it should be merged to REL-old (unless explicitly stated in the previous change). I'm not too firm on that for changes that it's obvious should be backported, such as a XSS fix*, but I would completely oppose to automerge a minor feature because it was merged into master. Note that we are not alone opinating about what it's worth backporting, since downstream distros will also call into question if our new release is “just bugfixes” before they agree into accepting it as-is. * Still, we could be making a complete new class in master but just stripping the vulnerable piece in the old release. ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] When to automerge (Re: Revoking +2 (Re: who can merge into core/master?))
On Feb 15, 2013, at 3:32 PM, Platonides platoni...@gmail.com wrote: I'm not convinced that backporting should be automatically merged, though. Even if the code at REL-old is the same as master (ie. the backport doesn't needs any code change), approving something from master is different than agreeing that it should be merged to REL-old (unless explicitly stated in the previous change). I'm not too firm on that for changes that it's obvious should be backported, such as a XSS fix*, but I would completely oppose to automerge a minor feature because it was merged into master. Note that we are not alone opinating about what it's worth backporting, since downstream distros will also call into question if our new release is “just bugfixes” before they agree into accepting it as-is. I don't know where you pull auto-merging from but it certainly isn't from my e-mail, which was about revoking merge access and about when self-merging may or may not be tolerated. Auto-merging would imply some random dude can take a change from master merged by someone else *for master*, and submit it to any branch and have it be auto-merged. What I was talking about is that a code reviewer with merge access can submit an approved change from master to another branch and self-merge it. Just because one can however doesn't mean one should. When our random dude pushes a change for review to an old branch that backports a feature from master, the assigned reviewer should (as you explain) not approve it. And for the same reason, when that reviewer backports himself, he wouldn't self-merge. Or rather, he wouldn't draft such a change in the first place. -- Krinkle ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] When to automerge (Re: Revoking +2 (Re: who can merge into core/master?))
On Fri, Feb 15, 2013 at 6:32 AM, Platonides platoni...@gmail.com wrote: On 15/02/13 01:38, Brad Jorsch wrote: I'd propose one more: * Someone else gives +2, but Jenkins rejects it because it needs a rebase that is not quite trivial enough for it to do it automatically. For example, something in RELEASE-NOTES-1.21. Seems a better example. I'm not convinced that backporting should be automatically merged, though. Even if the code at REL-old is the same as master (ie. the backport doesn't needs any code change), approving something from master is different than agreeing that it should be merged to REL-old (unless explicitly stated in the previous change). I'm not too firm on that for changes that it's obvious should be backported, such as a XSS fix*, but I would completely oppose to automerge a minor feature because it was merged into master. We should probably reset the subject again since this is something of a tangent from revoking +2, but since it was brought up, let me clarify the process behind when gerrit reports that someone is doing a self-merge for security fixes (and I welcome input for ways to improve the process!). When we get a report of an xss (assuming isn't not yet public or being actively exploited), we file a bugzilla ticket in the security category. Usually me or someone else in that group adds a patch to the bug. Someone else gives a yes, this looks ok to deploy comment on the bug. The patch is put into production, backported to the supported release versions, then we release tarballs / patches. When the tarballs are released, I typically submit and merge the patches into master and supported branches, since we have a number of users who pull from git to update their systems. This process is painful (no one like reviewing patches in bugzilla), and the wrangling to get the right people to review patches in bugzilla is slowing down our security releases. It would be much better if we had a way to submit the patches in gerrit, go through the normal review process by a trusted group of developers ending in a +2's, and then the final merge is just a single click when we release the tarballs. But we haven't been able to get gerrit to do that yet (although if any java developers want to work on that, I would be very excited). Note that we are not alone opinating about what it's worth backporting, since downstream distros will also call into question if our new release is “just bugfixes” before they agree into accepting it as-is. * Still, we could be making a complete new class in master but just stripping the vulnerable piece in the old release. ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] When to automerge (Re: Revoking +2 (Re: who can merge into core/master?))
On 15/02/13 18:51, Chris Steipp wrote: This process is painful (no one like reviewing patches in bugzilla), and the wrangling to get the right people to review patches in bugzilla is slowing down our security releases. It would be much better if we had a way to submit the patches in gerrit, go through the normal review process by a trusted group of developers ending in a +2's, and then the final merge is just a single click when we release the tarballs. But we haven't been able to get gerrit to do that yet (although if any java developers want to work on that, I would be very excited). gerrit drafts? Although those are not as private as we would like. Another option would be to email those amongst +2 reviewers, although willingness to review through email perhaps won't be bigger than in bugzilla. ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l