Upgrade to Pro — share decks privately, control downloads, hide ads and more …

How do I review the code

How do I review the code

All of a sudden I review more and more code while working for solarisBank as a Staff Engineer. This presentation is about getting the best out of code reviews and being a helpful teammate. ... and still, have fun from your job.

Denis Yagofarov

May 26, 2019
Tweet

More Decks by Denis Yagofarov

Other Decks in Technology

Transcript

  1. 05.06 /me • I’ve Wrote a lot of code in

    Ruby, Scala, and JavaScript • Seen some awesome and crazy things (in so@ware engineering) • Now working for solarisBank as an engineer • My team builds [payment] cards transacHon processing
  2. 05.06 self.talk • My own experience • Non-technical and completely

    subjec9ve • Use at your own risk • All stunts were done by me, but hey… I’m s9ll alive
  3. What kind of code? • Different teams • Different products

    • Different urgency • … and everything is connected !" #$ %& '( )*
  4. Why reviewing at all? • For sharing the knowledge •

    Learning and coaching • Discussing solu5ons async, close to the code • Keeping the codebase healthy • A regulatory requirement in
  5. Code in master brings value • Acceptance criteria should be

    covered. In tests • No questions regarding the syntax and style – it should be automated • Merge PR straight away if it’s good to go • … and I can always open a PR into the original one. Asking for feedback • Every approval is a tradeoff between "no value" and "added technical debt
  6. No software is perfect • Comment only about the most

    important things • Mark if the feedback s8ll lets merging the PR • … and approve merge, but yet list things to improve • I don’t always refer to “Boy-scout” rule • I leave comments even if PR is merged already
  7. Respecting people • My assump)ons: • The engineer did the

    best thing he or she could • And don’t make the same mistakes mul)ple )mes on purpose • And I don’t have a full context. Nor looked into all possibili)es ways of solving the problem • This means “I can be wrong in my judgments”: • Use benchmarks • Provide metrics (code quality, test coverage) • I ask ques)ons to get a full understanding of the context
  8. BTW, who had one of?.. !,",⁉ or $,% and &

    As an only comment to your PR
  9. Commenting and being clear • ! is ❌ a #$

    (yet) • Use simple English • And, yes, it’s OK to add Emoji too • Provide sample code. Not rewriAng a big part, however • Leave a final comment in PR, summarize your comments
  10. Save &me on async and sync communica&ons • When I

    comment, it’s now my responsibility too to clear it out • It may take just 10 minutes for two people to resolve a long-going conversation • … and add the recap into the PR
  11. Be positive • No ma&er, how “bad” it is, there

    should be something good. Find it and appreciate it.