Conversation

Notices

1. Embed this notice
   DeepBlue V7.X (deepbluev7@nheko.io)'s status on Wednesday, 20-Nov-2024 23:09:03 JST DeepBlue V7.X

   @charliermarsh@hachyderm.io I actually think there is a gradient there. There are issues that really should be fixed, things that you think are useful to discuss, nitpicks which could be ignored and things better left unsaid. Code reviews really are a social skill and need lots of practice and empathy, imo.

   • MortSinyx likes this.
   • Embed this notice
     Jeff Grigg (jeffgrigg@mastodon.social)'s status on Thursday, 21-Nov-2024 00:03:11 JST Jeff Grigg

     in reply to

     @deepbluev7 @charliermarsh

     It seems unfortunate that, as an industry, we've largely fallen into the rut that "code review" is "leaving comments on a pull request, some of which block the merge."

     The original code review research, which justifies the practice, was a *DISCUSSION* of the code being reviewed. Not asynchronous socially disconnected text comments.

     …

     MortSinyx likes this.
   • Embed this notice
     Jeff Grigg (jeffgrigg@mastodon.social)'s status on Thursday, 21-Nov-2024 00:03:56 JST Jeff Grigg

     in reply to

     @deepbluev7 @charliermarsh

     If a code review were being done as a *discussion between peers*, then it would be far more appropriate to say, "Hmmm… I was thinking I would have done it this other way. But are there advantages to this way?" And that opens up a discussion. A text comment, marked as a required "todo," blocking merge, is a challenge, not a discussion.

     During discussion, the reviewer might agree that what the submitter did was the best choice.

     MortSinyx likes this.