What a great recommendation! Thanks for sharing!
--stephen On Mon, Jun 29, 2015 at 10:51 AM, Toby Negrin <tneg...@wikimedia.org> wrote: > Thanks for posting this Sam -- code review is one of the best parts of our > engineering culture, I'll definitely watch this when I get a chance. > > -Toby > > On Mon, Jun 29, 2015 at 7:59 AM, Sam Smith <samsm...@wikimedia.org> wrote: > >> 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 >> >> >> _______________________________________________ >> Mobile-l mailing list >> Mobile-l@lists.wikimedia.org >> https://lists.wikimedia.org/mailman/listinfo/mobile-l >> >> > > _______________________________________________ > Mobile-l mailing list > Mobile-l@lists.wikimedia.org > https://lists.wikimedia.org/mailman/listinfo/mobile-l > >
_______________________________________________ Mobile-l mailing list Mobile-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mobile-l
