Reverting the commit will often be cheaper to do than fixing the
regression in a follow-on patch, *but there'll undoubtedly be exceptions,
which we'll deal with (and learn from) as a team.*
(Emphasis my own)
As you've both pointed out, there are indeed instances where reverting a
patch will be more expensive than simply fixing the regression in a
follow-on patch. As with everything else in our profession, it's a
judgement call, which require experience to make.
I certainly wasn't trying to introduce "if it's broke, revert it" as a
rule
either. We should be aspiring to have master always ready to be deployed.
Demanding that it's always deployable would be setting ourselves up for
failure.
Also is it a regression if the tests do not pick it up?
Yes. A regression isn't a tree in a forest.
We can't rely on tests to pick up regressions all the time. We don't have a
dedicated QA team to defer to either. We're left with slowing down and
being more deliberate in our functional review/sign off of patches. Taking
a day a week to do a QA pass of the site also isn't out of the question.
–Sam
On Fri, Jun 12, 2015 at 8:36 PM, Jon Robson <jdlrobson(a)gmail.com> wrote:
Sounds great Sam. I think the tricky thing is defining
what a regression
means. For instance, if you are fixing a regression and introducing a less
important regression then what? Master is technically undeployable in both
states.
Real world example:
Fixed to
https://phabricator.wikimedia.org/T98498 caused
https://phabricator.wikimedia.org/T102215
Also is it a regression if the tests do not pick it up?
It seems that if we pick up a regression 2 weeks later, it's not almost
the less sensible/feasible to revert it but I'm not sure.
Just stuff to think about. I agree in principle we need to be more
diligent and extreme when regressions happen.
On Fri, Jun 12, 2015 at 8:38 AM, Adam Baso <abaso(a)wikimedia.org> wrote:
Moving to mobile-l.
On Friday, June 12, 2015, Sam Smith <samsmith(a)wikimedia.org> wrote:
Hey web slingers,
If there is a regression introduced by a patch, then please revert that
patch as soon as you've identified it and let the team know via
Phabricator, email, or both. Reverting the commit will often be cheaper to
do than fixing the regression in a follow-on patch, but there'll
undoubtedly be exceptions, which we'll deal with (and learn from) as a team.
Fixing the regression in a follow-on patch means that:
- *master won't be deployable* until the patch has been reviewed,
tested, and merged, which should be communicated to the Release Engineering
team
- reviewers might have to drop what they're working on in order to
get it reviewed
- what if the original patch was lower priority?
- we should be cognisant of the cost of context switching
- the commit history will be dirty
*Master should always be depoyable.*
–Sam
_______________________________________________
Mobile-l mailing list
Mobile-l(a)lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mobile-l
--
Jon Robson
*
http://jonrobson.me.uk
*
https://www.facebook.com/jonrobson
* @rakugojon
_______________________________________________
Mobile-l mailing list
Mobile-l(a)lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mobile-l