On Wed, Feb 8, 2012 at 4:40 AM, Petr Bena benapetr@gmail.com 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
On Wed, Feb 8, 2012 at 9:17 PM, Rob Lanphier robla@wikimedia.org 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
On Wed, Feb 8, 2012 at 12:17 PM, Rob Lanphier robla@wikimedia.org 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
On Wed, Feb 8, 2012 at 6:43 PM, Brion Vibber brion@pobox.com 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
On 02/08/2012 06:59 PM, Chad wrote:
On Wed, Feb 8, 2012 at 6:43 PM, Brion Vibber brion@pobox.com 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.
On Wed, Feb 8, 2012 at 3:43 PM, Brion Vibber brion@pobox.com 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@lists.wikimedia.org