3 ms·

I'm going slightly off-topic here, but I have to rant a bit on code reviews. For the love of god, please, they are meant for checking each others code for mist

I'm going slightly off-topic here, but I have to rant a bit on code reviews. For the love of god, please, they are meant for checking each others code for mistakes and not for miniscule details like this. I have seen it time and time again in various companies where code review is used as some sort of tool to dictate style and preferences to each other. Wasting valuable development time and creating a unhealthy tension between developers.

vowelless 13y ago
While I agree with you in general, in this particular case, we can run into problems when compiling the code on different platforms. Many teams/companies probably don't have a need for that. But I have been on cross platform projects where such errors would lead to days of wasted debugging time.
deleted 13y ago
[deleted]
aleyan 13y ago
If code reviewers were just for checking mistakes, then unit tests would be enough. Code reviews are invaluable in education other developers about your code, how to support it and how to develop on it. But information should also flow the other way and the give the reviewers the opportunity to present the accepted practices to the developer. In the long term, this saves developer time as the code base stays coherent. Code is read many times and needs to be understood by many developers even though it is written only once.
lotsofcows 13y ago
Ironically, that's the exact opposite to the way it works in my company. We use various automated techniques to pick up mistakes and use code review to ensure style is consistent. Everyone in the company should be able to parse any piece of code without individual styles disrupting that. The only tension occurs around the weighty question of line length - coding standard says 76 chars max, I say that's what the IDE's for.
- SoftwareMaven 13y ago
  I think 76 characters is a good ideal to strive for because it means the code is legible everywhere, including my phone. It also reduces variables named request_response_header_validating_auditor. However, some things don't break well, so readability ought to trump.
danielweber 13y ago
If you are compiling on multiple platforms, then stuff like this is a disaster. If you are not compiling on multiple platforms, then complaining about stuff like this because of compiler issues is a nuisance. (You could still object on the basis that it is fugly.)
Evbn 13y ago
Spraying horrible garbage cleverness in code, and submitting commented out code, are exactly the sort of newbie mistakes that code reviews should sort out. At best, this gimmick is for hacking out some temp code while debugging, and should not be submitted. Of course, one should just use an IDE, or even vim, to commenting out code as needed.

vowelless 13y ago

While I agree with you in general, in this particular case, we can run into problems when compiling the code on different platforms. Many teams/companies probably don't have a need for that. But I have been on cross platform projects where such errors would lead to days of wasted debugging time.

deleted 13y ago

[deleted]

aleyan 13y ago

If code reviewers were just for checking mistakes, then unit tests would be enough. Code reviews are invaluable in education other developers about your code, how to support it and how to develop on it. But information should also flow the other way and the give the reviewers the opportunity to present the accepted practices to the developer. In the long term, this saves developer time as the code base stays coherent. Code is read many times and needs to be understood by many developers even though it is written only once.

lotsofcows 13y ago

Ironically, that's the exact opposite to the way it works in my company. We use various automated techniques to pick up mistakes and use code review to ensure style is consistent. Everyone in the company should be able to parse any piece of code without individual styles disrupting that. The only tension occurs around the weighty question of line length - coding standard says 76 chars max, I say that's what the IDE's for.

SoftwareMaven 13y ago
I think 76 characters is a good ideal to strive for because it means the code is legible everywhere, including my phone. It also reduces variables named request_response_header_validating_auditor. However, some things don't break well, so readability ought to trump.

danielweber 13y ago

If you are compiling on multiple platforms, then stuff like this is a disaster. If you are not compiling on multiple platforms, then complaining about stuff like this because of compiler issues is a nuisance. (You could still object on the basis that it is fugly.)

Evbn 13y ago

Spraying horrible garbage cleverness in code, and submitting commented out code, are exactly the sort of newbie mistakes that code reviews should sort out. At best, this gimmick is for hacking out some temp code while debugging, and should not be submitted. Of course, one should just use an IDE, or even vim, to commenting out code as needed.