5 ms·

Completely agree. I think the flaw is that someone believes they turn up to a project and in reviewing code to fit it to a mental model they understand they be

Completely agree. I think the flaw is that someone believes they turn up to a project and in reviewing code to fit it to a mental model they understand they believe they're improving the readability for others who think like them. In reality it becomes design by committee, worst of all worlds. Slightly tangential, but I'm about to leave a project where it gets the review process badly wrong - anyone can and will comment on a review, but only a select few can merge. They have a massive bottleneck of reviews with many held up by conflicting style preferences, or comments on neighbouring code not modified, none of which is able to be resolved by agreement as no one willing to take authority over any one view. Even the simplest one line change can't get merged within a month which essentially means development has stalled other than for the few who have merge rights. Give me a lone developer's codebase to wrestle with any day.

klabb3 4y ago
> I'm about to leave a project where it gets the review process badly wrong - anyone can and will comment on a review, but only a select few can merge. Ugh, I definitely recognize this. There is often a lack of management behind those things. My heuristic is: If I can’t do my job, I tell my manager and ask for help. If they can’t fix it, there’s no point in going on.
matwood 4y ago
> They have a massive bottleneck of reviews with many held up by conflicting style preferences Stuff like this is so frustrating. And it's not only the reviewers fault. I don't consider myself a great coder, but a skill I do have is being able to fit my changes into the codes existing style. Consistency trumps any partial incremental improvement. Unless I have the time and desire to fix the entire code base, new code needs to fit in with the old. For some reason, I've seen many programmers have a hard time with fitting the style of the existing system. This makes every small change slightly different from others and over time makes the entire system harder and harder to reason about.
mordae 4y ago
> In reality it becomes design by committee, worst of all worlds. And then there is the special hell of projects written by groups of hurried physicists, zoologists, linguists or other non-IT scientists. And below that, heirloom university software projects where every generation builds on the previous iteration, without ever cleaning it up.
ted_bunny 4y ago
Why isn't a "when in Rome" attitude more common? Personally I'd feel obligated to copy the existing style, if it's not my project.
pc86 4y ago
I've been places where this has happened, but it's usually a fairly easy fix. Two things that are usually not that controversial: 1) Comments on unmodified code are either deleted/ignored or you create a ticket for the actual issue (and whoever made the comment is reminded this isn't helpful or appropriate in a review). 2) Nitpicky/stylistic things are handed via prettier, precommit hooks, etc. so that everything is consistent and largely doesn't need to be thought about. And the potentially controversial one: anyone can merge into [almost] any branch, and you merge your own code - so quite often you'll get a PR approval and might wait a day or two to merge it until you decide it's ready. We let anyone merge into any branch but our production branch, however that gets merged automatically and deployed daily unless it's proactively cancelled.

klabb3 4y ago

> I'm about to leave a project where it gets the review process badly wrong - anyone can and will comment on a review, but only a select few can merge. Ugh, I definitely recognize this. There is often a lack of management behind those things. My heuristic is: If I can’t do my job, I tell my manager and ask for help. If they can’t fix it, there’s no point in going on.

matwood 4y ago

> They have a massive bottleneck of reviews with many held up by conflicting style preferences Stuff like this is so frustrating. And it's not only the reviewers fault. I don't consider myself a great coder, but a skill I do have is being able to fit my changes into the codes existing style. Consistency trumps any partial incremental improvement. Unless I have the time and desire to fix the entire code base, new code needs to fit in with the old. For some reason, I've seen many programmers have a hard time with fitting the style of the existing system. This makes every small change slightly different from others and over time makes the entire system harder and harder to reason about.

mordae 4y ago

> In reality it becomes design by committee, worst of all worlds. And then there is the special hell of projects written by groups of hurried physicists, zoologists, linguists or other non-IT scientists. And below that, heirloom university software projects where every generation builds on the previous iteration, without ever cleaning it up.

ted_bunny 4y ago

Why isn't a "when in Rome" attitude more common? Personally I'd feel obligated to copy the existing style, if it's not my project.

pc86 4y ago

I've been places where this has happened, but it's usually a fairly easy fix. Two things that are usually not that controversial: 1) Comments on unmodified code are either deleted/ignored or you create a ticket for the actual issue (and whoever made the comment is reminded this isn't helpful or appropriate in a review). 2) Nitpicky/stylistic things are handed via prettier, precommit hooks, etc. so that everything is consistent and largely doesn't need to be thought about. And the potentially controversial one: anyone can merge into [almost] any branch, and you merge your own code - so quite often you'll get a PR approval and might wait a day or two to merge it until you decide it's ready. We let anyone merge into any branch but our production branch, however that gets merged automatically and deployed daily unless it's proactively cancelled.