date:20110602

Re: [Wikitech-l] Code review process (was: Status of more regular code deployments)

2011-06-02 Thread Ariel T. Glenn

Στις 01-06-2011, ημέρα Τετ, και ώρα 15:58 -0600, ο/η bawolff έγραψε:
On Wed, Jun 1, 2011 at 3:02 PM, Brion Vibber br...@pobox.com wrote:
On Wed, Jun 1, 2011 at 1:53 PM, bawolff bawolff...@gmail.com wrote:

As a volunteer person, I'm fine if code I commit is reverted based on
it sucking, being too complicated, being too ugly, etc provided there
is actually some objection to the code. However, I'd be rather
offended if it was reverted on the basis of no one got around to
looking at in the last 3 days, since that is something essentially out
of my control.

When someone looks at your commit within ~72 hours and reverts it because
nobody's yet managed to figure out whether it works or not and it needs more
research and investigation... what was the reason for the revert?

Because 'no one reviewed it'? Or because someone looked at it and decided it
needed more careful review than they could give yet?

-- brion

That's fine. I'm only concerned with the possibility of no one looks
at the commit, and it gets summarily reverted (Which is what I thought
was being proposed).

--bawolff

One question is how thorough is thorough: if the code is reviewed for
security issues and doesn't apparently break things, does it need to get
gone over with a fine toothed comb before we accept it for deployment?
I would say no, and that we could live with the commit adheres to our
basic coding guidelines, looks like it will generally do what it says
(without necessarily having tested every detail personally), and follows
general usage patterns (appropriate use of hooks, cacheing, i18n, etc.
etc.).

Can we ensure that folks have (now and ongoing) enough available time to
review commits to trunk at that level within 72 hours? This means
buy-in from reviewers *and from their managers* that they have X time
for this and that time will not get reassigned to other priorities.

If we can't ensure this, we need to decide on the minimum level of
review we *can* get done, or extend the time period, or both.

Asking all committers to trunk to guess whether their reviewer can or
cannot make the 72-hour window for their commit seems like a recipe for
pissing people off.

Just my two cents.

Ariel

P.S. Oh and for the record I would like 4 releases a year ideally, I
could live with 3, and I would be pretty unhappy with two. Deployment
from trunk should happen a lot more often. I don't know if we can
manage weekly, but it's a good target.

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] Code review process (was: Status of more regular code deployments)

2011-06-02 Thread Robert Leverington

Thank you for posting this Erik.

I initially didn't reply on this thread because to me it just seems to
be the same as the 3 or 4 times we've had the same discussion before.
No technical or procedural change is going to solve a problem that is
ultimately caused by a lack of time assigned to it.  I was somewhat
disappointed by the suggestion that the onus should be on volunteers to
find a reviewer for their code, as this is just as unworkable without
staff time assigned to doing it - and perhaps could be even more
demotivating than the status quo.

Having review be a portion of staff time was always how I assumed it
would be arranged.  The justification for not assigning time to review
has often been that one person or team doing it all the time would
result in burn-out, and I agree with this.  As a (part-time) developer I
certainly wouldn't want to spend all my time doing service work, even
for just one or two weeks at a time.  The idea of having a single
person/team assigned for each deploy/release cycle is nice, but I think
that it would lead to burn-outs and conflicts with existing
responsibilities those people have.  Whatever model is implemented, I
hope it can finally be successful.

I reject the notion that we need a change in our review processes.
Empirical evidence shows that when staff time is assigned to it, our
code review system *does* work.  (Of course there are always minor
changes that would improve efficiency, such as automating some of the
reporting tasks that have traditionally been done manually.)

Updates on how this is going would be nice, as again it took MZMcBride
to bring it up before anyone senior posted about it - and this is still
the big issue for volunteer developers.

Cheers,
Robert

On 2011-06-01, Erik Moeller wrote:
 On Wed, Jun 1, 2011 at 4:50 PM, Brion Vibber br...@pobox.com wrote:
  This is one of the reasons I tend to advocate shorter
  development/review/deployment cycles. By keeping the cycle short, we can
  help build up regular habits: run through some reviews every couple days. Do
  a deployment update *every* week. If you don't think your code will be
  working within that time, either work on it outside of trunk or break it up
  into pieces that won't interfere with other code.
 
  With a long cycle, review gets pushed aside until it's no longer habit, and
  gets lost.
 
 Right. And just to weigh in quickly on the resources issue -- the
 review/deploy/release train is clearly not moving at the pace we want.
 This does affect WMF staffers and contractors as well, but we know
 that it's especially frustrating for volunteers and third party
 committers. We kicked around the idea of a 20% rule for all funded
 engineers (IMO not just senior staff) in Berlin and in the office
 yesterday, and I think Roan mentioned it earlier in this thread: i.e.
 ensuring that every WMF-funded engineer spends one day a week on
 service work (code review, design/UX review, deployment, shell bugs,
 software bugs, bug triaging, etc.).
 
 An alternative model is to have rotating teams that do this work. I
 personally prefer the 20% model because it gives more
 consistency/predictability and less churn, but I'm curious what other
 folks think, and I'm comfortable with us continuing this discussion
 openly on this list.
 
 Whether that would get us to a healthier balance remains to be seen,
 but I think there's pretty broad agreement that adding more support to
 the review/deployment/release process is a necessary precondition for
 any other process changes like moving towards pre-commit review.
 
 Clearly what's been said in this thread is true -- there are lots of
 things that can be done to reduce our technical debt and make it
 easier to accommodate and manage new changes, but without added
 dedicated capacity, the train won't move at a speed that we're happy
 with. We can't change that overnight (because we need to figure out
 the right rhythm and the right processes together), but we will change
 it.
 
 Erik
 -- 
 Erik Möller
 Deputy Director, Wikimedia Foundation
 
 Support Free Knowledge: http://wikimediafoundation.org/wiki/Donate
 
 ___
 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

1 2 3 >

1 - 100 of 204 matches

Mail list logo