924
you are viewing a single comment's thread
view the rest of the comments
[-] aeki@slrpnk.net 94 points 8 months ago

I am really happy when people are quite strict in code reviews, it makes me feel safer and I get to learn more.

Nothing worse than some silent approvals with no real feedback. What if I missed something obvious... and now it's merged.

To be fair, I also enjoy getting my grammar corrected. I'm juggling 3 languages and things can get messy.

[-] jadero@programming.dev 33 points 8 months ago

In that spirit, I will call attention to your first sentence, specifically the comma. In my opinion, that can be improved. One of three other constructions would be more appropriate:

  • I am really happy when people are quite strict in code reviews. It makes me feel safer and I get to learn more.
  • I am really happy when people are quite strict in code reviews, because it makes me feel safer and I get to learn more.
  • I am really happy when people are quite strict in code reviews; it makes me feel safer and I get to learn more.

The first of my suggested changes is favoured by those who follow the school of thought that argues that written sentences should be kept short and uncomplicated to make processing easier for those less fluent. To me, it sounds choppy or that you've omitted someone asking "Why?" after the first sentence.

Personally, I prefer the middle one, because it is the full expression of a complete state of mind. You have a feeling and a reason for that feeling. There is a sense in which they are inseparable, so not splitting them up seems like a good idea. The "because" explicitly links the feeling and reason.

The semicolon construction was favoured by my grade school teachers in the 1960s, but, as with the first suggestion, it just feels choppy. I tend to overuse semicolons, so I try to go back and either replace them with periods or restructure the sentences to eliminate them. In this particular case, I think the semicolon is preferable to both comma and period, but still inferior to the "because" construction.

I've clearly spent too much time hashing stuff out in writers' groups. :)

[-] little_hermit@lemmus.org 6 points 8 months ago

Strange, I get a mild hostility vibe from colleagues if I review too ambitiously.

[-] jadero@programming.dev 2 points 8 months ago

Reviews have to be balanced to circumstance. There is a big difference between putting out the sales brochure and the notice on the bulletin board. Likewise in coding a cryptographic framework for general consumption and that little script to create personal slideshows based on how you've tagged your photos.

As a general rule, wider distributions, public distributions, and long-lived distributions need more ambitious reviews. If the distribution is wide, public, and permanent, then everything needs very detailed scrutiny.

I have found some success in starting with and occasionally revisiting review goals. This helps create and maintain some consistency in a process that is scaled to the task at hand.

load more comments (2 replies)
load more comments (20 replies)
this post was submitted on 26 Oct 2023
924 points (96.5% liked)

Programmer Humor

18253 readers
1 users here now

Welcome to Programmer Humor!

This is a place where you can post jokes, memes, humor, etc. related to programming!

For sharing awful code theres also Programming Horror.

Rules

founded 1 year ago
MODERATORS