[Wikitech-l] When to automerge (Re: Revoking +2 (Re: who can merge into core/master?))

2013-02-15 Thread Platonides
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?))

2013-02-15 Thread Krinkle
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?))

2013-02-15 Thread Chris Steipp
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?))

2013-02-15 Thread Platonides
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