Save 37% off PRO during our Black Friday Sale! »

Code Reviews

Code Reviews

When working in the cold and calculating space of computers, it can be easy to forget the emotions of our fellow colleagues. This is especially true when reviewing someone else's work. We need to be sensitive to one's time, effort, and thought while also providing constructive feedback. Emoticons can help express the connotation of feedback you are giving which is sometimes lost in written form. This talk discusses how to use emoticons to set the tone and provide clear visual indication for the context of your code review feedback.

E0e036f89c14b3e59640318eedf9670b?s=128

Brooke Kuhlmann

August 10, 2021
Tweet

Transcript

  1. Code Reviews Presented by Brooke Kuhlmann

  2. https://www.alchemists.io/articles/peopleware Peopleware

  3. "When a group of people fuse into a meaningful whole,

    the entire character of the work changes." https://www.alchemists.io/articles/peopleware Peopleware
  4. "When a group of people fuse into a meaningful whole,

    the entire character of the work changes." https://www.alchemists.io/articles/peopleware Peopleware "A jelled team is a group of people so strongly knit that the whole is greater than the sum of the parts."
  5. None
  6. TRUST

  7. TRUST 🔨 🔧 🪛

  8. Building Trust

  9. Building Trust 👩💻 💡 Ideas Work ❤ Compassion

  10. Insensitive Feedback

  11. Insensitive Feedback No. This can't possibly work. I don't agree

    with these changes. This is unacceptable. You might as well do this over.
  12. Considerate Feedback

  13. Considerate Feedback Nice work and thanks for folding in the

    feedback. Have you thought about the security implications? What about using the following solution? Would you want to use the following instead? Maybe leverage this aspect of the architecture instead?
  14. Feedback Categories Blocking Non-blocking https://www.alchemists.io/articles/code_reviews

  15. Feedback Categories Blocking Non-blocking • Requires immediate attention. https://www.alchemists.io/articles/code_reviews

  16. Feedback Categories Blocking Non-blocking • Requires immediate attention. • Must

    be fi xed. https://www.alchemists.io/articles/code_reviews
  17. Feedback Categories Blocking Non-blocking • Requires immediate attention. • Must

    be fi xed. • Informative or suggestive. https://www.alchemists.io/articles/code_reviews
  18. Feedback Categories Blocking Non-blocking • Requires immediate attention. • Must

    be fi xed. • Informative or suggestive. • Author's discretion. https://www.alchemists.io/articles/code_reviews
  19. Feedback Categories Blocking Non-blocking • Requires immediate attention. • Must

    be fi xed. • Informative or suggestive. • Author's discretion. • Changes are encouraged. https://www.alchemists.io/articles/code_reviews
  20. Feedback Categories Blocking Non-blocking • Requires immediate attention. • Must

    be fi xed. • Informative or suggestive. • Author's discretion. • Changes are encouraged. • Discussion can continue. https://www.alchemists.io/articles/code_reviews
  21. Feedback Format https://www.alchemists.io/articles/code_reviews

  22. Feedback Format <emoticon> <message>. https://www.alchemists.io/articles/code_reviews

  23. Feedback Format <emoticon> <message>. 🎨 Would you want to use

    `Object#then` to avoid the temporary local variable? https://www.alchemists.io/articles/code_reviews
  24. Emoticons https://www.alchemists.io/articles/code_reviews

  25. Emoticons ℹ 💭 💡 ⚠ 🎨 🛑 🙇 ❇ 📛

    🪓 🍄 https://www.alchemists.io/articles/code_reviews
  26. Emoticons ℹ 💭 💡 ⚠ 🎨 🛑 🙇 ❇ 📛

    🪓 🍄 https://www.alchemists.io/articles/code_reviews
  27. Emoticons ℹ 💭 💡 ⚠ 🎨 🛑 🙇 ❇ 📛

    🪓 🍄 https://www.alchemists.io/articles/code_reviews
  28. Emoticons ℹ 💭 💡 ⚠ 🎨 🛑 🙇 ❇ 📛

    🪓 🍄 https://www.alchemists.io/articles/code_reviews Encouraging/Informational
  29. Emoticons ℹ 💭 💡 ⚠ 🎨 🛑 🙇 ❇ 📛

    🪓 🍄 https://www.alchemists.io/articles/code_reviews Encouraging/Informational Concerning/Actionable
  30. Emoticons ℹ 💭 💡 ⚠ 🎨 🛑 🙇 ❇ 📛

    🪓 🍄 https://www.alchemists.io/articles/code_reviews
  31. Emoticons ℹ 💭 💡 ⚠ 🎨 🛑 🙇 ❇ 📛

    🪓 🍄 https://www.alchemists.io/articles/code_reviews Blues / Whites
  32. Emoticons ℹ 💭 💡 ⚠ 🎨 🛑 🙇 ❇ 📛

    🪓 🍄 https://www.alchemists.io/articles/code_reviews Yellows
  33. Emoticons ℹ 💭 💡 ⚠ 🎨 🛑 🙇 ❇ 📛

    🪓 🍄 https://www.alchemists.io/articles/code_reviews Reds
  34. Emoticons ℹ 💭 💡 ⚠ 🎨 🛑 🙇 ❇ 📛

    🪓 🍄 https://www.alchemists.io/articles/code_reviews Greens (mixed)
  35. Information ℹ • Non-blocking. • Status update. • Generally doesn't

    need a response. https://www.alchemists.io/articles/code_reviews
  36. Information ℹ • Non-blocking. • Status update. • Generally doesn't

    need a response. :information_source: https://www.alchemists.io/articles/code_reviews
  37. Information (examples) ℹ

  38. Information (examples) ℹ Will rebase onto `main` once build is

    green. ℹ Rebasing to resolve upstream merge con fl icts. ℹ All feedback has been addressed and ready for re-review. ℹ
  39. Inquire 💭 • Non-blocking. • A chance to learn more.

    https://www.alchemists.io/articles/code_reviews
  40. Inquire 💭 • Non-blocking. • A chance to learn more.

    :thought_balloon: https://www.alchemists.io/articles/code_reviews
  41. Inquire (examples) 💭

  42. 💭 We should consider leveraging the Command Pattern for these

    objects soon. 💭 Since this pattern exhibits The Rule of Three, should we plan to DRY this up soon? Inquire (examples) 💭
  43. Tip 💡 • Non-blocking or blocking. • A chance to

    educate. https://www.alchemists.io/articles/code_reviews
  44. Tip 💡 • Non-blocking or blocking. • A chance to

    educate. :bulb: https://www.alchemists.io/articles/code_reviews
  45. Tip (examples) 💡

  46. 💡You might want to study the use of whole value

    objects when it comes to immutability. 💡You might enjoy looking at existing examples of pattern matching. Tip (examples) 💡
  47. Style 🎨 • Non-blocking or blocking. • Improvements to style.

    https://www.alchemists.io/articles/code_reviews
  48. Style 🎨 • Non-blocking or blocking. • Improvements to style.

    :art: https://www.alchemists.io/articles/code_reviews
  49. Style (examples) 🎨

  50. 🎨 Would it be easier to inject your dependencies to

    avoid a tight coupling of these constants? 🎨 Could you use `%()` to avoid escaping quotes in this string? Style (examples) 🎨
  51. Warning ⚠ • Non-blocking or blocking. • Alert or caution.

    https://www.alchemists.io/articles/code_reviews
  52. Warning ⚠ • Non-blocking or blocking. • Alert or caution.

    :warning: https://www.alchemists.io/articles/code_reviews
  53. Warning (examples) ⚠

  54. ⚠ You'll need to rebase in order to resolve upstream

    merge con fl icts based on the work Jake recently added. ⚠ Be aware that the work Jill is doing will soon require these objects to be updated. Warning (examples) ⚠
  55. Typo 📛 • Blocking. • Corrects a mispelling. https://www.alchemists.io/articles/code_reviews

  56. Typo 📛 • Blocking. • Corrects a mispelling. :name_badge: https://www.alchemists.io/articles/code_reviews

  57. Typo (examples) 📛

  58. 📛 In the fi rst sentence of your commit message,

    I think you mean to say "capital" instead of "capitol". 📛 I think you want to say "implemented" instead of "implemed". Typo (examples) 📛
  59. Cut 🪓 • Blocking. • Signi fi es text, code,

    or commit that can be deleted. https://www.alchemists.io/articles/code_reviews
  60. Cut 🪓 • Blocking. • Signi fi es text, code,

    or commit that can be deleted. :axe: https://www.alchemists.io/articles/code_reviews
  61. Cut (examples) 🪓

  62. 🪓 No need to keep this object around anymore since

    it's not called anywhere else in the code base. 🪓 You can delete this spec since it's a duplicate of Line 12. Cut (examples) 🪓
  63. Architecture 🛑 • Blocking. • Signi fi es overarching architectural

    concerns. https://www.alchemists.io/articles/code_reviews
  64. Architecture 🛑 • Blocking. • Signi fi es overarching architectural

    concerns. :stop_sign: https://www.alchemists.io/articles/code_reviews
  65. Architecture (examples) 🛑

  66. 🛑 Could you break this object down into smaller objects

    since the specs are so complex and trying to tell you as much? 🛑 Could you inject all of these constants instead of embedding them? This would allow you to have an object that is more fl exible to change. Architecture (examples) 🛑
  67. Bonus 🍄 • Non-blocking. • Next level power up. https://www.alchemists.io/articles/code_reviews

  68. Bonus 🍄 • Non-blocking. • Next level power up. :mushroom:

    https://www.alchemists.io/articles/code_reviews
  69. Bonus (examples) 🍄

  70. 🍄 There are several other places in the codebase that

    would bene fi t from this object. Reducing duplication would be a nice win as part of this work. 🍄 It would be awesome if you could take this work a step further and refactor all of the other objects in this namespace to use the same pattern. Bonus (examples) 🍄
  71. Favorite ❇ • Non-blocking. • Positive only. • Must be

    authentic. https://www.alchemists.io/articles/code_reviews
  72. Favorite ❇ • Non-blocking. • Positive only. • Must be

    authentic. :sparkle: https://www.alchemists.io/articles/code_reviews
  73. Favorite (examples) ❇

  74. ❇ Looking forward to rebasing upon what you've done here

    since it'll make my job a lot easier! ❇ Excellent work folding in all of the feedback. Favorite (examples) ❇
  75. Thanks 🙇 • Non-blocking. • A chance to honor feedback.

    https://www.alchemists.io/articles/code_reviews
  76. Thanks 🙇 • Non-blocking. • A chance to honor feedback.

    :bow: https://www.alchemists.io/articles/code_reviews
  77. Thanks (examples) 🙇

  78. 🙇 Fixed. I meant to remove that commit but forgot.

    🙇 Fixed. Good eye. 🙇 Fixed. Nice catch. Thanks (examples) 🙇
  79. Resolving Feedback • Address quickly. • Split into smaller stories

    as necessary. https://www.alchemists.io/articles/code_reviews
  80. Cadence https://www.alchemists.io/articles/code_reviews

  81. Cadence 🌄 ☀ 🌃 Dawn Noon Dusk https://www.alchemists.io/articles/code_reviews

  82. Guidelines https://www.alchemists.io/articles/code_reviews

  83. Guidelines • Lines of Code: ~300 lines (max) https://www.alchemists.io/articles/code_reviews

  84. Guidelines • Lines of Code: ~300 lines (max) • Length

    of Time: ~1 day (max) https://www.alchemists.io/articles/code_reviews
  85. Recap https://www.alchemists.io/articles/code_reviews

  86. Recap • Use emoticons. 🎉 https://www.alchemists.io/articles/code_reviews

  87. Recap • Use emoticons. 🎉 • Be considerate of other's

    work, ideas, and time. https://www.alchemists.io/articles/code_reviews
  88. Recap • Use emoticons. 🎉 • Be considerate of other's

    work, ideas, and time. • Always provide constructive feedback. https://www.alchemists.io/articles/code_reviews
  89. Recap • Use emoticons. 🎉 • Be considerate of other's

    work, ideas, and time. • Always provide constructive feedback. • Learn from one another and level up. https://www.alchemists.io/articles/code_reviews
  90. 🙇 Thanks https://www.alchemists.io