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

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

June 28, 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?
  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` here in order 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. ℹ Rebased to resolve upstream merge con fl icts. ℹ All feedback has been addressed and is 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. 💭 It would be nice to leverage the Command Pattern

    for these objects soon. 💭 Can this duplicate code be extracted at some point? 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. 💡If you splatted all arguments in the array, you'd achieve

    the same outcome. 💡You might want to use `Object#tap` to handle this side e ff ect. 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. 🎨 If you injected your dependencies, you'd not have such

    tight coupling of constants. 🎨 If you used `%()`, you'd be able to avoid escaping your quotes. 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 merge

    con fl icts and not break the build. ⚠ 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. 🛑 If you injected your dependencies, you'd be able to

    compose more complex objects while simplifying your specs. 🛑 You might want to consider using `String#tr` since it has signi fi cant performance bene fi ts. 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. Refactoring those objects and reducing code duplication would be a nice win as part of this work. 🍄 Taking this a step further and refactoring all of the other objects in this namespace to use the same pattern introduced here would be awesome. 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. 🙇 Good point, I meant to remove that commit but

    forgot. Fixed. 🙇 Good eye. Fixed. 🙇 Nice catch. Fixed. 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: ~250 lines (max) https://www.alchemists.io/articles/code_reviews

  84. Guidelines • Lines of Code: ~250 lines (max) • Lenght

    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