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. How do I review the code
    Denys Yahofarov May 2019

    View Slide

  2. 05.06
    /me
    • I’ve Wrote a lot of code in Ruby, Scala, and
    JavaScript
    • Seen some awesome and crazy things (in
    [email protected] engineering)
    • Now working for solarisBank as an engineer
    • My team builds [payment] cards transacHon
    processing

    View Slide

  3. 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

    View Slide

  4. What kind of code?
    • Different teams
    • Different products
    !"
    #$
    %&
    '(
    )*

    View Slide

  5. What kind of code?
    • Different teams
    • Different products
    • Different urgency
    • … and everything is connected
    !"
    #$
    %&
    '(
    )*

    View Slide

  6. 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

    View Slide

  7. My principles

    View Slide

  8. The code brings value in master,
    deployed to production*
    * Unless it’s an PoC

    View Slide

  9. No software is perfect

    View Slide

  10. Contributors deserve respect.
    No ma3er who they are

    View Slide

  11. Think twice before posting a comment

    View Slide

  12. Time is precious. Going back and forth
    with comments is painful

    View Slide

  13. No matter how “bad” the code is, there
    should be something good

    View Slide

  14. 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

    View Slide

  15. 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

    View Slide

  16. 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

    View Slide

  17. BTW, who had one of?..
    !,",⁉ or
    $,% and &
    As an only comment to your PR

    View Slide

  18. 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

    View Slide

  19. 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

    View Slide

  20. Be positive
    • No ma&er, how “bad” it is, there should be
    something good. Find it and appreciate it.

    View Slide

  21. ”I’d better do it myself” - does not scale

    View Slide

  22. Act the same way you expect other
    people to act

    View Slide

  23. What do
    you think?

    View Slide

  24. Thank You.

    View Slide