4 ms·

After 30+ years, I kind of reverse the pyramid, honestly. I look for readable, maintainable code, and try to make sure the writer is doing appropriate testing.

After 30+ years, I kind of reverse the pyramid, honestly. I look for readable, maintainable code, and try to make sure the writer is doing appropriate testing. But whether or not it works and is correct, that's not my job, that's the developer's job. I know developers that will spend days on a code review, which is a horrible waste of time. CI/CD and your deployment followup structure (canaries, monitors, gates, etc) should be catching any significant issues the vast majority of the time, and if it isn't, you need to spend time there. My main concern as a reviewer is to make sure future people can understand the code when the inevitable modification comes along. Things are different if the writer and reviewer are in a mentorship relationship, but even still, if you are only engaging with the code as a mentor when the PR hits, you're messing up the mentorship!

beerpls 3y ago
“ I look for readable, maintainable code, and try to make sure the writer is doing appropriate testing. But whether or not it works and is correct, that's not my job, that's the developer's job.” And I personally refuse to work with this style of workflow anymore Does the code do what it needs? That’s objective and it either does or doesn’t. I can manage that kind of review. Does the code look pretty enough? This is subjective and guarantees i’ll waste hours of my life every month bc someone else felt anxious/neurotic/masochistic and wanted to dump a chore on me. No thanks, if you have style guidelines i’ll follow them. I’m not your whipping boy that will ask how high when you say jump.
- bnjm 3y ago
  > Does the code look pretty enough? ‘…readable, maintainable [and tested]’, ie. if someone else needs to modify or remove this code in the future, will there be friction?
- smokel 3y ago
  This is a bit harsh. The fact that something is subjective does not mean that it is useless. IMHO, writing maintainable code requires a lot of experience, which one cannot expect from all junior developers. Requiring style guidelines for everything is a bit pedantic, and does not even make sense when you stride in to new territories. The latter is increasingly becoming the only part of software development where I would like to go. If all code and requirements would be so objective and so simple, then I'm afraid you'll soon be replaced by an AI agent :)
- gilbetron 3y ago
  Readability is not "prettiness", it's minimizing the effort as much as feasible to understand what the code is doing. I don't care what lines the braces are on.

beerpls 3y ago

“ I look for readable, maintainable code, and try to make sure the writer is doing appropriate testing. But whether or not it works and is correct, that's not my job, that's the developer's job.” And I personally refuse to work with this style of workflow anymore Does the code do what it needs? That’s objective and it either does or doesn’t. I can manage that kind of review. Does the code look pretty enough? This is subjective and guarantees i’ll waste hours of my life every month bc someone else felt anxious/neurotic/masochistic and wanted to dump a chore on me. No thanks, if you have style guidelines i’ll follow them. I’m not your whipping boy that will ask how high when you say jump.

bnjm 3y ago
> Does the code look pretty enough? ‘…readable, maintainable [and tested]’, ie. if someone else needs to modify or remove this code in the future, will there be friction?
smokel 3y ago
This is a bit harsh. The fact that something is subjective does not mean that it is useless. IMHO, writing maintainable code requires a lot of experience, which one cannot expect from all junior developers. Requiring style guidelines for everything is a bit pedantic, and does not even make sense when you stride in to new territories. The latter is increasingly becoming the only part of software development where I would like to go. If all code and requirements would be so objective and so simple, then I'm afraid you'll soon be replaced by an AI agent :)
gilbetron 3y ago
Readability is not "prettiness", it's minimizing the effort as much as feasible to understand what the code is doing. I don't care what lines the braces are on.