Feedback from CW22

[bielsnohr]

This is feedback from group two (Meet and Agree on Objectives):

To avoid a scatter of too many issues, I will collect the feedback from my breakout room here, and we can decide if they each warrant an individual issue

• The use of the word "extensibility" should be made clearer in the context of code and perhaps should be replaced with "reusability"
  • There was a valid point about whether this means it is easy to extend the original code, or whether it means that it is easy to build more code on top of the original code
• Variety of typos (did this in a cheeky push to main branch)
• Include some mention about whether a code review should address "code correctness"
  • I think we have fairly good agreement that the main purpose of a code review is not to check if the code is producing the correct results; that is the domain of testing
  • However, there was a good point that catching code which is likely to cause "unintended consequences" in the future is something that a code review can do, and in this more limited respect, checking for "correctness" is possible. This is again a bit of a semantic issue, depending on what you mean by "correctness". This again emphasises the importance of having a startup meeting and hashing out these definitions and expectations.
• Should async review be the default way to do code review?
  • feeling was that async allows for more time to think
  • sync could be done when it is more convenient (i.e. when author and reviewer in same group/department)
• Acknowledge that setting objectives won't necessarily be that firm
  • in particular, one participant noted that the reviewer might not be able to fulfil the objectives of the review because there are aspects of the code that prevent this (e.g. the code being very unreadable would prevent looking at anything else)
• Lean on writing examples rather than trying to get the perfect description

[NickleDave]

co-opting this issue (sorry @bielsnohr ) to make a single "meta"-issue that includes feedback / takeaways / to-dos for all rooms as discussed at our last meeting

Notes from that meeting:

• Holly, room 1 "find a reviewer" takeaways
  • what is "moral of the story" from the examples
  • link to RSE slack channels as last resort in "lone coder"?
  • too much technical language
    • e.g. "language" should be "programming language"
    • For instance we (I?) use the word "idiomatic" a lot...
  • TL: we had the same feedback -- either not use these words, or add a glossary
  • TL: hover on a word to get definition
  • MB: Carpentries glossary?
    • link to / DN: can we recycle some of that?
    • https://glosario.carpentries.org/en/ <-- link (ty HM)
  • various typos
• @bdemchak / "perform code review"
  • david barged in to discuss other issues (at beginning of "author and reviewer meet" section: move "sync/async" box, remove, or re-frame #68 and in "author and reviewer meet" section: include language about cultural differences #69)
  • re: at beginning of "author and reviewer meet" section: move "sync/async" box, remove, or re-frame #68 there are ppl that would prefer async
  • DN: our approach is to suggest in-person
  • MB: yes but we can mention async exists
  • TB: in-person is more of a recommendation
    • we need to explain on the website what you can gain in person and what you may lose in email / Slack
    • you can only discuss technical stuff; more difficult to tal about the human side of the code review
  • BD: be able to pick up human emotions -- if you don't know the person
  • should have a dedicated sync v async blurb (MB)
    • BD: cover all the "pros and cons"
    • TL: also talk @ diffs b/t in-person v "virtual meeting", zoom as we know from pandemic; cues that are not easy to pick up on etc
    • DN: start out sync -- then can move to async mode, once you establish relationship
    • MB: agreed as someone who was introduced to code review would “evolve” from sync to async
• other feedback
  • HM: lots of interest
  • BD: def a place for async and in-person
  • BD: also make friends
  • HM: building LTR with a review partner
• @tlestang group: main takeaways:
  • too wordy
    • try to have less text, shorted sentences, break it down in diff't paragraphs,
      especially in a way that makes it easier to reference
  • glossary, def need to add that back
  • language re: cultural differences (covered by in "author and reviewer meet" section: include language about cultural differences #69 but we probably need to mention this several places)
  • can we have a summary version?
    • that we try to do on high-level flowchart :/