All these comments about how code review is a waste of time, or suggest code review is only for bugs, really shine light on why so much software is incredibly slow today.
Improving code review time
121–130 of 233 posts
Re: Improving code review time
#122If code is important enough, it will get reviewed and tested one way or another.
Everything else is a waste of time.
Re: Improving code review time
#123I'm a strong believer in fast reviews. I get into deep working mode for 3 hours a day total, on a good day. The rest is meetings, daily sync, coffee, lunch, "hey can you look at something", hallway conversations, emails, my own inability to concentrate when I'm not feeling it. I've been in this industry for coming up on ten years. None of this is going to change, unless I become an academic or a hermit. I don't get t…
Variation on 2 - We don't care about commit messages at all. All PRs must have good descriptions which use a company-wide template. But individual commits get squashed and their commit messages entirely replaced by the PR template contents.
So my commit history is
"fump" "GAH" "I think this works?"
etc. but all changes have a description of the issue (and link back to the ticket) the solution described in some detail.
Re: Improving code review time
#124Earlier quoted context omitted.
If only it were that simple. As we’ve seen in this thread, you’d instead likely have: Somebody else: Where I used to work we called them pull requests. Let’s have a meeting to discuss changing the terms to match industry standards. We may also want to create a committee to agree on other terminology changes too, like main instead of master.
I'm not sure if you're saying this as an argument for standardisation (ha, let's debate the spelling) or not, but these people will always find something, from frameworks to linting rules - the amount of discussion stays the same no matter how much is agreed
I'm not really arguing for one way or the other, just pointing out the reality of working with lots of people means lots of opinions on how to do things, so it's never that straightforward to get things done. It's just a fact of life.
And yeah, I've been in those long discussions too about frameworks and linting rules. It can often turn into just egos trying to win and less about doing what's right or expedient for the team.
Re: Improving code review time
#125Earlier quoted context omitted.
Meta had the best code review quality I've seen in my career.
As someone who has only experienced terrible code reviews, could you give a little summary of what you liked?
Calling you out for lazy work. Teaching you how to use internal tools, company coding standards, suggesting better patterns, digging deep in to the context of system design and maintainability, suggesting other domain experts to tag in the reviews. I learned all sorts of Hack things that obviously I wouldn't learn outside from code reviews.
Re: Improving code review time
#126Re: Improving code review time
#127Earlier quoted context omitted.
> I take my name off the reviewer list and it’s reassigned. How big is your team? We've got 4 developers on our team (and one lead), and require two signoffs for code changes. There's just not enough of us for something like that to work... right?
That doesn't sound like the best ratio to me, but I don't know what you are running or how good your deploy process is at catching issues. With a 4 person team, you are probably better off discussing risky/bigger changes as a whole team, and allowing 1 review for smaller, less risky changes with well defined rollback processes.
Also with a team so small you can't afford wasting it on meetings.
You might need focus, i.e. non-meeting days, 3 days a week.
Re: Improving code review time
#128Earlier quoted context omitted.
If you're working 8 hours and do at least 4 breaks per day - that's a break every 2 hours. If you do a break you might as well skim through emails and do some reviews. And remember that everything* is Pareto distributed, the review request sizes too. It's really not hard at all. [*] it's an exaggeration, but close to reality.
> If you do a break you might as well skim through emails and do some reviews. That's not exactly a break...
Well, after or before the break. The point is you're going to lose context completely anyhow.
And I'd like to stress the distribution of the "complexity of a review", which you can crudely approximate with something like number of lines changed. Most of them will be small.
Re: Improving code review time
#129Earlier quoted context omitted.
I quite agree with the proposition you are making here. It goes to the very heart of the matter, does it not. The better you are at giving detailed, explicit and concrete feedback about each and every particular aspect of a diff (NOTE: both good and bad), the better and more competent you clearly are, and the more of a true champion for the cause you are proving to be. Time and time again, I wish my reviewers would j…
Meh. I've often seen reviewers delivering essays to back up their arguments, which essentially boil down to "because I prefer it this way". Focusing on pure word count doesn't mean that the feedback is valid, or even explains the reasoning well. If anything, it encourages nitpicky and long winded comments based on personal preference. Often, less is more. If you can get a point across by a small code suggestion, do t…
I think you're replying to a joke comment about inflating word count via loquacious reviews. But I'm not sure either, so kudos to the author if it is.
Re: Improving code review time
#130I'm a strong believer in fast reviews. I get into deep working mode for 3 hours a day total, on a good day. The rest is meetings, daily sync, coffee, lunch, "hey can you look at something", hallway conversations, emails, my own inability to concentrate when I'm not feeling it. I've been in this industry for coming up on ten years. None of this is going to change, unless I become an academic or a hermit. I don't get t…
Variation on 2 - We don't care about commit messages at all . All PRs must have good descriptions which use a company-wide template. But individual commits get squashed and their commit messages entirely replaced by the PR template contents. So my commit history is "fump" "GAH" "I think this works?" etc. but all changes have a description of the issue (and link back to the ticket) the solution described in some detai…
Is there any tooling you use to squash the commits and use the PR message?