Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Wikipedia: Wikitech

How to avoid a post-branch code slush (Re: Branch 1.19 on Tuesday, February 6?)

 

 

Wikipedia wikitech RSS feed   Index | Next | Previous | View Threaded


robla at wikimedia

Feb 8, 2012, 12:17 PM

Post #1 of 6 (379 views)
Permalink
How to avoid a post-branch code slush (Re: Branch 1.19 on Tuesday, February 6?)

On Wed, Feb 8, 2012 at 4:40 AM, Petr Bena <benapetr [at] gmail> wrote:
> Hi, is there any update on branching? Thank you

Sam will be doing it soon.

After that it means, in theory, that trunk will be open for
post-deploy commits. However, we *cannot* let the same backlog back
up that we did before, and there's no way we can keep up with
everything we need to do for deployment (last minute bugfixes,
addressing fixmes regardless of committer) while at the same time
dealing with a flood of new commits.

A big problem with our current post-commit review regime is that it is
exactly these times that really regrettable changes can and probably
do get made. Many refactoring exercises happen without much
discussion on the mailing list. The code doesn't get reviewed, and
then it gets entangled with a lot of other important code to the point
that we're forced to forge ahead with a suboptimal refactoring
decision. In addition to building up a large review backlog, we also
find ourselves chasing pockets of breakage due in part to incomplete
refactoring and backwards-incompatibility breakages.

We're migrating to Git very soon after this release. It would really
suck to have a huge pile of unreviewed commits going into trunk. So,
I'm going to suggest a Git migration strategy that will avoid having a
monsterous backlog. Instead of cutting over trunk at the very latest
revision, we cut over at the latest revision that is fully reviewed
and ok. Everything before the 1.19 branch point would be
grandfathered in, but everything after would need to be reviewed and
ok. So, for example, if r111000 is the branch point, and
r111000-r111020 are reviewed, but r111021 is a huge omnibus change
that sits unreviewed or fixme'd, then r111020 would be the branch
point, even if r111022-r120000 are fully reviewed. That's hopefully
an extreme example, but the goal is to make sure that trunk is always
fully reviewed.

What would happen to everything after the branch point is tbd. I
haven't talked to Chad about this, but I think it's conceivable that
we can import the remainder of commits into a branch that we can
cherrypick.

The dynamic that this will create is that it will motivate more
peer-to-peer scrutiny of code, rather than waiting for one of the
reviewers to play bad cop.

Until we agree on this strategy or some other strategy that everyone
agrees is workable, we'll need to keep the code slush in place.

Thanks
Rob

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


roan.kattouw at gmail

Feb 8, 2012, 3:14 PM

Post #2 of 6 (363 views)
Permalink
Re: How to avoid a post-branch code slush (Re: Branch 1.19 on Tuesday, February 6?) [In reply to]

On Wed, Feb 8, 2012 at 9:17 PM, Rob Lanphier <robla [at] wikimedia> wrote:
> We're migrating to Git very soon after this release.  It would really
> suck to have a huge pile of unreviewed commits going into trunk.  So,
> I'm going to suggest a Git migration strategy that will avoid having a
> monsterous backlog.  Instead of cutting over trunk at the very latest
> revision, we cut over at the latest revision that is fully reviewed
> and ok.  Everything before the 1.19 branch point would be
> grandfathered in, but everything after would need to be reviewed and
> ok.  So, for example, if r111000 is the branch point, and
> r111000-r111020 are reviewed, but r111021 is a huge omnibus change
> that sits unreviewed or fixme'd, then r111020 would be the branch
> point, even if r111022-r120000 are fully reviewed.  That's hopefully
> an extreme example, but the goal is to make sure that trunk is always
> fully reviewed.
>
That sounds good to me, in general terms. I hope that in practice we
can be a bit flexible so that if we could move the cutover revision up
by dozens of revisions by spending a few hours reviewing, fixing or
reverting a couple of things, we do it (for instance, if r111021 is
not a nightmare but just controversial and not too hard to revert).

> What would happen to everything after the branch point is tbd.  I
> haven't talked to Chad about this, but I think it's conceivable that
> we can import the remainder of commits into a branch that we can
> cherrypick.
>
That could be done, but will be hard if reviewed commits depend on
unreviewed ones. We could also just submit all post-cutoff commits
into Gerrit individually but still depending on each other. We can
then rubber-stamp the ones that were approved in CR before, and Gerrit
won't actually merge such a revision unless and until the previous
revision is approved. Submitting a long chain of dependent commits
like that should work quite nicely, *provided that none of them are
amended*: if you amend one commit, you have to rebase all of its
successors. That means that normal Gerrit review procedures wouldn't
apply to these commits and changes made to satisfy reviews would have
to be separate commits. We also couldn't rebase independent, approved
commits against master to get them merged more quickly, because that
would have the same effect.

Possibly, we could cherry-pick everything that can be cherry-picked,
and submit the other commits into Gerrit after rebasing them. That
might be more or less work than submitting everything into Gerrit,
depending on specifics.

Roan

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


brion at pobox

Feb 8, 2012, 3:43 PM

Post #3 of 6 (357 views)
Permalink
Re: How to avoid a post-branch code slush (Re: Branch 1.19 on Tuesday, February 6?) [In reply to]

On Wed, Feb 8, 2012 at 12:17 PM, Rob Lanphier <robla [at] wikimedia> wrote:

> After that it means, in theory, that trunk will be open for
> post-deploy commits. However, we *cannot* let the same backlog back
> up that we did before, and there's no way we can keep up with
> everything we need to do for deployment (last minute bugfixes,
> addressing fixmes regardless of committer) while at the same time
> dealing with a flood of new commits.
>
> A big problem with our current post-commit review regime is that it is
> exactly these times that really regrettable changes can and probably
> do get made. Many refactoring exercises happen without much
> discussion on the mailing list. The code doesn't get reviewed, and
> then it gets entangled with a lot of other important code to the point
> that we're forced to forge ahead with a suboptimal refactoring
> decision.


This is one of the reasons I've been hoping we'd move to a more pre-commit
review model. Especially for big refactorings and cleanups that have
limited immediate value, we tend to get a lot of breakages a not a lot of
interest in fully reviewing them (eg actually checking all the code paths
to make sure they really work).

To a certain degree, I'd actually consider it desirable to have a permanent
'slush' to the extent that destabilizing work should *always* be talked out
a bit and tested before it lands on trunk/head/master.

If we're not ready to go fully git the instant we branch 1.19, we may wish
to consider applying more formal review to things proposed to go into trunk
on SVN. This may be simpler than attempting to synchronize SVN and git via
post-SVN-pre-git reviews...

-- brion
_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


innocentkiller at gmail

Feb 8, 2012, 3:59 PM

Post #4 of 6 (369 views)
Permalink
Re: How to avoid a post-branch code slush (Re: Branch 1.19 on Tuesday, February 6?) [In reply to]

On Wed, Feb 8, 2012 at 6:43 PM, Brion Vibber <brion [at] pobox> wrote:
> To a certain degree, I'd actually consider it desirable to have a permanent
> 'slush' to the extent that destabilizing work should *always* be talked out
> a bit and tested before it lands on trunk/head/master.
>

+100

> If we're not ready to go fully git the instant we branch 1.19, we may wish
> to consider applying more formal review to things proposed to go into trunk
> on SVN. This may be simpler than attempting to synchronize SVN and git via
> post-SVN-pre-git reviews...
>

I'm inclined to agree here too.

-Chad

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


sumanah at wikimedia

Feb 8, 2012, 4:31 PM

Post #5 of 6 (362 views)
Permalink
Re: How to avoid a post-branch code slush (Re: Branch 1.19 on Tuesday, February 6?) [In reply to]

On 02/08/2012 06:59 PM, Chad wrote:
> On Wed, Feb 8, 2012 at 6:43 PM, Brion Vibber <brion [at] pobox> wrote:
>> If we're not ready to go fully git the instant we branch 1.19, we may wish
>> to consider applying more formal review to things proposed to go into trunk
>> on SVN. This may be simpler than attempting to synchronize SVN and git via
>> post-SVN-pre-git reviews...
>>
>
> I'm inclined to agree here too.
>
> -Chad

Public service announcement: if you regularly commit code to MediaWiki
or MediaWiki extensions and you rarely or never review code, we will
help you learn to review so you can help widen the bottleneck (and
become a better developer). See
https://www.mediawiki.org/wiki/Code_review_guide for some tips and catch
any of the core developers on IRC, e.g., during WMFers' 20% time
https://www.mediawiki.org/wiki/Wikimedia_engineering_20%25_policy , for
guidance and training.

--
Sumana Harihareswara
Volunteer Development Coordinator
Wikimedia Foundation

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


robla at wikimedia

Feb 8, 2012, 10:23 PM

Post #6 of 6 (350 views)
Permalink
Re: How to avoid a post-branch code slush (Re: Branch 1.19 on Tuesday, February 6?) [In reply to]

On Wed, Feb 8, 2012 at 3:43 PM, Brion Vibber <brion [at] pobox> wrote:
> This is one of the reasons I've been hoping we'd move to a more pre-commit
> review model. Especially for big refactorings and cleanups that have
> limited immediate value, we tend to get a lot of breakages a not a lot of
> interest in fully reviewing them (eg actually checking all the code paths
> to make sure they really work).

Here's the thinking that lead to where we are: the cutover to Git is
the point at which we want to fully move to precommit. It seems like
an enormous pain-in-the-butt to move to full precommit with our
current toolset (SVN + CodeReview tool).

However, we're much closer than we've ever been to having Git, and it
may be worth dealing with some short-term pain.

> To a certain degree, I'd actually consider it desirable to have a permanent
> 'slush' to the extent that destabilizing work should *always* be talked out
> a bit and tested before it lands on trunk/head/master.

Yup, agree 100%.

Let's all just pretend this has always been the status quo starting
right now. I think we've already established that there used to be
more liberal reversion, and that when that went away, so too went our
ability to stay on top of the review queue.

> If we're not ready to go fully git the instant we branch 1.19,

Given that the branch just happened, and we're not ready yet, that's
the case. Chad can give you more of an update, but my understanding
is:
* A (hopefully) final test migration of core is slated for this week.
Chad believes he's got all of the blocking problems sorted out.
* Extensions migration isn't going so smoothly. The same tools that
work splendidly with core seem to crash with the very small subset of
extensions that he's tried it out with. Could be a minor problem
that's easy to fix, or could be gawd awful. TBD
* We'd like a two-week window of warning/testing/playing around
before making the cutover.

All told, the current plan is beginning of March for core, middle of
March for extensions. More details here:
https://www.mediawiki.org/wiki/Git/Conversion

...and in the email that I hear Chad is writing :-)

> we may wish
> to consider applying more formal review to things proposed to go into trunk
> on SVN. This may be simpler than attempting to synchronize SVN and git via
> post-SVN-pre-git reviews...

I'd be perfectly fine with either outcome (more formal pre-commit
review, or picking our SVN->Git cutover point based on what's
reviewed).

Rob

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Wikipedia wikitech RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.