This brings back memories of https://opendev.org/ttygroup/gertty when I was contributing to OpenStack
Code review can be better
181–190 of 253 posts
Re: Code review can be better
#182What bothered me for a long time with code reviews is that almost all useful things they catch (i.e. not nit-picking about subjective minor things that doesn't really matter) are much too late in the process. Not rarely the only (if any) useful outcome of a review is that everything has to be done from scratch in a different ways (completely new design) or that it is abandoned since it turns out it should never have…
This can be used in any process where the result is only judged at the end.
The solution here may be to add a midterm check. I think this is what you mean by a "design review."
In my experience, there are some rules that need to be followed for it to work.
- Keep the number of stakeholders involved in all decisions, including PR, as small as possible.
- Everyone involved should take part in this check. That way, no one will be surprised by the results.
- This check should have been documented, like in the ticket.
This can be used in any process where the result is only judged at the end. The solution here may be to add a midterm check. I think this is what you mean by a "design review." In my experience, there are some rules that need to be followed for it to work. We should keep the number of stakeholders involved in all decisions, including PR, as small as possible. Everyone involved should take part in this mid-term check. That way, no one will be surprised by the results. This check should have been documented, like in the ticket.
When and how to do this check and how to handle disagreements depend on the task, culture, and personalities.
Re: Code review can be better
#183I'm of the hot opinion that a reviewer shouldn't be running code. The one making the code is responsible for the fix, code reviews are just about maintainability.
If your PR did not fix the issue or implement the feature, that's on you, not the reviewer.
Re: Code review can be better
#184[dead]
Re: Code review can be better
#185Earlier quoted context omitted.
Personally I don't need to talk with "traditional" engineers to have an opinion there, as I am mechanical engineer that currently deals mostly with software, but still in the context of "traditional" engineering (models and simulation, controls design). Definitely making software can be engineering , most of the time it is not, not because of the nature of software, but the characteristics of the industry and culture…
Engineering is just about wielding tools to solve problems. You don't need to use formal methods to do engineering in general. Sometimes they're useful; sometimes they're required; often they just get in the way. In the context of software vs other sub-disciplines, the big difference is in the cost of iterating and validating. A bridge has very high iteration cost (generally, it must be right first time) and validati…
> Engineering is just about wielding tools to solve problems.
By that standard, doctors and hair stylists are also engineers, as are some chimps and magpies. I don't think it's a useful definition, it's far too broad.
Re: Code review can be better
#186What bothered me for a long time with code reviews is that almost all useful things they catch (i.e. not nit-picking about subjective minor things that doesn't really matter) are much too late in the process. Not rarely the only (if any) useful outcome of a review is that everything has to be done from scratch in a different ways (completely new design) or that it is abandoned since it turns out it should never have…
I share your feelings. Regarding design reviews, we used to have them at my current job. However we stopped doing both formal design documents and design reviews in favor of prototyping and iterative design. The issue with the design phase is that we often failed to account for some important details. We spent considerable time discussing things and, when implementing, realized that we omitted some important detail o…
> The ideal setup is to put 5 people in the same room with the PO and close to a few key users.
(I suspect you are aware, but just in case this is new to you.) This is essentially the core of Extreme Programming.
Re: Code review can be better
#187What bothered me for a long time with code reviews is that almost all useful things they catch (i.e. not nit-picking about subjective minor things that doesn't really matter) are much too late in the process. Not rarely the only (if any) useful outcome of a review is that everything has to be done from scratch in a different ways (completely new design) or that it is abandoned since it turns out it should never have…
This can be used in any process where the result is only judged at the end. The solution here may be to add a midterm check. I think this is what you mean by a "design review." In my experience, there are some rules that need to be followed for it to work. - Keep the number of stakeholders involved in all decisions, including PR, as small as possible. - Everyone involved should take part in this check. That way, no o…
We should do something similar with AI-coding.
If you don't have a documented mid-term check, vibe-coded PR might not be what you expected.
Re: Code review can be better
#188What bothered me for a long time with code reviews is that almost all useful things they catch (i.e. not nit-picking about subjective minor things that doesn't really matter) are much too late in the process. Not rarely the only (if any) useful outcome of a review is that everything has to be done from scratch in a different ways (completely new design) or that it is abandoned since it turns out it should never have…
I work in a small team where we are essentially 4-6 core developers. When I develop a feature I usually talk about it with my coworkers once I made a rough draft in my head how I'd approach it. They do the same so our code reviews are mostly only the minor code smells etc. but we usually decide on the architecture together (2-3 people usually). I find this to be one of the most important things in our team. Once peop…
I strongly second this. In my own experience of about 30 years, I have seen this method to work almost always.
Re: Code review can be better
#189I'm of the hot opinion that a reviewer shouldn't be running code. The one making the code is responsible for the fix, code reviews are just about maintainability. If your PR did not fix the issue or implement the feature, that's on you, not the reviewer.
I don't know about "shouldn't", I think it's fine if they do. But I basically agree, at some fundamental level, you have to have some trust in your coworkers. If someone says "This fixes X", and they haven't even tried running it or testing it, they shouldn't be your coworker. The purpose of code reviews shouldn't be "is this person honest?" or "is this person totally incompetent?". If they're not, it's a much bigger issue, one that shouldn't be dealt with through code reviews.
Very different situation if it's open source or an external contribution, of course.
Re: Code review can be better
#190I'm of the hot opinion that a reviewer shouldn't be running code. The one making the code is responsible for the fix, code reviews are just about maintainability. If your PR did not fix the issue or implement the feature, that's on you, not the reviewer.
The author mentioned that he doesn't want to make suggestions that don't actually work. That seems like a pretty valid reason to run the code.