[teampractices] [TPG/URGENT-ISH] Thoughts on Gerrit Cleanup Day

[Joaquin Oltra Hernandez]

I would suggest doing a post-release day (branch cuts happen on Tuesday, so
maybe use Wed or Thu) so that if we merge a lot of patches we'll have as
much time as possible until the next release cut to test stuff on the beta
cluster.

This is a great idea, and we need to think of continuous efforts to educate
wmf developers about better code review practices.

I recommend reading this email Sam (phuedx) sent in June to mobile-l:
https://lists.wikimedia.org/pipermail/mobile-l/2015-June/009357.html

And watch the talk if you are interested.

Hey y'all,
> I watch a lot of talks in my downtime. I even post the ones I like to a
> Tumblr… sometimes [0]. I felt like sharing Derek Prior's "Implementing a
> Strong Code Review Culture" from RailsConf 2015 in particular because it's
> relevant to the conversations that the Reading Web team are having around
> process and quality. You can watch the talk on YouTube [1] and, if you're
> keen, you can read the paper that's referenced over at Microsoft Research
> [2].
> I particularly like the challenge of providing two paragraphs of context
> in a commit message – to introduce the problem and your solution – and
> trying to overcome negativity bias in written communication* by offering
> compliments whenever possible and asking, not telling, while providing
> critical feedback.
> I hope you enjoy the talk as much as I did.
> –Sam
> [0]Â http://sometalks.tumblr.com/
> [1]Â https://www.youtube.com/watch?v=PJjmw9TRB7s
> [2]Â http://research.microsoft.com/apps/pubs/default.aspx?id=180283
> * The speaker said "research has shown" but I didn't see a citation
>


*Notes (width added emphasis)*
>
>    - Code review isn't for catching bugs
>
>
>    - "Expectations, Outcomes, and Challenges of Modern Code Review"
>
>
>    - Chief benefits of code review:
>
>
>    - Knowledge transfer
>
>
>    - Increased team awareness
>
>
>    - Finding alternative solutions
>
>
>    - Code review is "the discipline of explaining your code to your peers"
>
>
>    - Process is more important than the result
>
>
>    - Goes on to define code review as "the discipline of discussing your
>    code with your peers"
>
>
>    - If we get better at code review, then we'll get better at
>    communicating technically as a team
>
> Rules of Engagement
>
>    - As an author, provide context
>
>
>    - "If content is king, then context is God"
>
>
>    - *In a pull request (patch set) the code is the content and the
>    commit message is the context*
>
>
>    - Provide sufficient context - bring the reviewer up to speed with
>    what you've been doing in the past X hours
>
>
>    - *Challenge: provide at least two paragraphs of context in your
>    commit message*
>
>
>    - This additional context lives on in the commit history whereas links
>    to issue trackers might not
>
>
>    - As a reviewer, ask questions rather than making demands
>
>
>    - Research has shown that there's a negativity bias in written
>    communication. *Offer compliments whenever you can*
>
>
>    - *When you need to provide critical feedback, ask, don't tell*, e.g.
>    "extract a service to reduce some of this duplication" could be formulated
>    as "what do you think about extracting a service to reduce some of this
>    duplication?"
>
>
>    - "Did you consider?", "can you clarify?"
>
>
>    - "Why didn't you just..." is framed negatively and includes the word
>    just
>
>
>    - Use the Socratic method: asking and answering questions to stimulate
>    critical thinking and to illuminate ideas
>
> Insist on high quality reviews, but agree to disagree
>
>    - Conflict is good. *Conflict drives a higher standard of coding
>    provided there's healthy debate*
>
>
>    - Everyone has a minimum bar to entry for quality. Once that bar is
>    met, then everything else is a trade-off
>
>
>    - Reasonable people disagree all the time
>
>
>    - Review what's important to you
>
>
>    - SRP (Single Responsibility Principle) (the S from SOLID)
>
>
>    - Naming
>
>
>    - Complexity
>
>
>    - Test Coverage
>
>
>    - ... (whatever else you're comfortable in giving feedback on)
>
>
>    - What about style?
>
>
>    - Style is important
>
>
>    - "People who received style comments on their code perceived that
>    review negatively"
>
>
>    - Adopt a styleguide
>
>
> Benefits of a Strong Code Review Culture
>
>    - Better code
>
>
>    - Better developers through constant knowledge transfer
>
>
>    - Team ownership of code, which leads to fewer silos
>
>
>    - Healthy debate
>
>

On Wed, Aug 5, 2015 at 1:58 AM, Arthur Richards <arichards at wikimedia.org>
wrote:

> Generally looks good to me.
>
> Couple of questions - are there plans to document/track 'unmaintained'
> repositories? Further, are there additional metrics that can come out of
> the GCD that we don't get out of Korma that would be useful information to
> have (eg patches to non-WMF maintained repos vs patches to WMF maintained
> repos)?
>
> On Tue, Aug 4, 2015 at 1:45 PM, Quim Gil <qgil at wikimedia.org> wrote:
>
>>
>>
>> On Tue, Aug 4, 2015 at 10:21 PM, David Strine <dstrine at wikimedia.org>
>> wrote:
>>
>>> How do we come to consensus on the proposed date?
>>>
>>
>> There is no day of the week where Asia, Europe, and America can work 8
>> hours together comfortably, so we need to think on an activity that makes
>> sense for a "Day" considering multiple timezones.
>>
>> I think Release Management and Engineering managers will prefer to stop
>> regular activities to focus on code review more on a Friday than any other
>> day, with release trains and the big bunch of regular team meetings
>> populating schedules. This is why I support a Friday, but will support
>> whatever alternative day of the week makes happy to Engineering teams.
>>
>> Again, it is an experiment, so we can pick a day and then analyze. We
>> won't find the day that works well for everybody.  :)
>>
>> --
>> Quim Gil
>> Engineering Community Manager @ Wikimedia Foundation
>> http://www.mediawiki.org/wiki/User:Qgil
>>
>> _______________________________________________
>> teampractices mailing list
>> teampractices at lists.wikimedia.org
>> https://lists.wikimedia.org/mailman/listinfo/teampractices
>>
>>
>
>
> --
> Arthur Richards
> Team Practices Manager
> [[User:Awjrichards]]
> IRC: awjr
> +1-415-839-6885 x6687
>
> _______________________________________________
> teampractices mailing list
> teampractices at lists.wikimedia.org
> https://lists.wikimedia.org/mailman/listinfo/teampractices
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.wikimedia.org/pipermail/teampractices/attachments/20150805/694c4e0a/attachment.html>