In over two years at WMF I have never been involved in a discussion like this, but here goes:
In this case, I think it was entirely appropriate to revert immediately and pick up the pieces later. The source of the code is immaterial, if Tim Starling or Brion Vibber had merged this we would have done exactly the same thing. As Steven noted, the immediate issue was that it created a serious problem with the mobile account creation process. This blocked our ability to test other aspects of mobile account creation and login that have changed recently. And, since this occurred on Thursday morning in the run-up to the weekly deployment, we had little time to prevent this going live to production. Beyond that, there are serious concerns with any feature that a) requires javascript support in the client in order to create an account on the wiki and b) does not honor the characters that the user types in the username and password fields. I know of at least one historical instance where violating b) caused a significant problem in UniversalLanguageSelector. We prevented the ULS problem from going live to production at the time, also. -Chris On Thu, Mar 6, 2014 at 1:29 PM, Tyler Romeo <tylerro...@gmail.com> wrote: > Hi everybody, > > I cannot believe I have to say something about this, but I guess it's no > surprise. > > Wikipedia has a notorious policy against edit warring, where users are > encouraged to discuss changes and achieve consensus before blindly > reverting. This applies even more so to Gerrit, since changes to software > have a lot bigger effect. > > Here's a nice example: > https://gerrit.wikimedia.org/r/114400 > https://gerrit.wikimedia.org/r/117234 > https://gerrit.wikimedia.org/r/117247 > > Some key points to note here: > * The revert commit was not linked to on the original commit > * The time between the revert patch being uploaded and +2ed was a mere two > minutes > * All the reviewers on the revert patch were also reviewers on the original > patch > > This is unacceptable behavior, and is extremely disrespectful to the > developers here. If you are going to revert a patch for reasons other than > a blatant code review issue (such as a fatal error or the likes), you > should *at the very least* give the original patch reviewers time to > understand why the patch is being reverted and give their input on the > matter. Otherwise it defeats the entire point of the code review process > and Gerrit in the first place. > > The argument being made in this specific case is that the change broke the > workflow of mobile, and that the revert was announced on mobile-l. This is > not sufficient for a number of reasons: > > 1) not everybody is subscribed to mobile-l, so you cannot expect the > original reviewers to see or know about it > 2) this is an issue with MobileFrontend, not MediaWiki core > 3) code being merged does not automatically cause a deployment, and if code > being deployed breaks something in production, it is the operations team's > job to undeploy that change > > Overall, the lesson to take away here is to be more communicative with > other developers, especially when you are negating their changes or > decisions. > > Thanks in advance, > *-- * > *Tyler Romeo* > Stevens Institute of Technology, Class of 2016 > Major in Computer Science > _______________________________________________ > 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
