| ▲ | tyrust 4 days ago |
| Code review is rarely done live. It's usually asynchronous, giving the reviewer plenty of time to read, digest, and give considered feedback on the changes. Perhaps a spicy patch would involve some kind of meeting. Or maybe in a mentor/mentee situation where you'd want high-bandwidth communication. |
|
| ▲ | throwaway314155 4 days ago | parent | next [-] |
| My first job did IRL code reviews with at least two senior devs in the loop. It was both devastating and extremely helpful. |
| |
| ▲ | SoftTalker 4 days ago | parent [-] | | Yeah when we first started, "code review" was a weekly meeting of pretty much the entire dev team (maybe 10 people). Not all commits were reviewed, it was random and the developer would be notified a couple of days in advance that his code was chosen for review so that he could prepare to demo and defend it. | | |
| ▲ | necovek 4 days ago | parent [-] | | Wow, that's a very arbitrary practice: do you remember roughly when was that? I was in a team in 2006 where we did the regular, 2-approve-code-reviews-per-change-proposal (along with fully integrated CI/CD, some of it through signed email but not full diffs like Linux patchsets, but only "commands" what branch to merge where). | | |
| ▲ | SoftTalker 4 days ago | parent | next [-] | | Around that time frame. We had CI and if you broke the build or tests failed it was your job to drop anything else you were doing and fix it. Nothing reached the review stage unless it could build and pass unit tests. | | |
| ▲ | necovek 4 days ago | parent [-] | | Right, we already had both: pre-review build & test runs, and pre-merge CI (this actually ran on a temp, merged branch). |
| |
| ▲ | marwamc 4 days ago | parent | prev [-] | | This was still practice at $BIG_FINANCE in the couple of years just before covid, although by that point such team reviews were reducing in importance and prominence. |
|
|
|
|
| ▲ | jopsen 4 days ago | parent | prev [-] |
| Doing only IRL code reviews would certainly improve quality in some projects :) It's probably also fairly expensive to do. |
| |
| ▲ | jghn 4 days ago | parent | next [-] | | Am old enough that this was status quo for part of my career, and have also been in some groups that did this as a rejection of modern code review techniques. There are pros & cons to both sides. As you point out it's quite expensive in terms of time to do the in person style. Getting several people together is a big hassle. I've found that the code reviews themselves, and what people get out of them, are wildly different though. In person code reviews have been much more holistic in my experience, sometimes bordering on bigger picture planning. And much better as a learning tool for other people involved. Whereas the diff style online code review tends to be more focused on the immediate concerns. There's not a right or wrong answer between those tradeoffs, but people need to realize they're not the same thing. | |
| ▲ | Ekaros 3 days ago | parent | prev | next [-] | | I would guess that 3 part code review would actually be most effective. Likely even save on costs. First part is walkthrough on call, next independent review and comments. Then per need an other call over fixes or discussion. Probably spend more time on it, but would share the understanding and alignment. | |
| ▲ | comprev 4 days ago | parent | prev | next [-] | | Pair programming? That is realtime code review by another human | |
| ▲ | stephen_cagle 4 days ago | parent | prev | next [-] | | And yet... is it? Realtime means real discussion, and opportunity to align ever so slightly on a common standard (which we should write down!), and an opportunity to share tacit knowledge. It also increases the coverage area of code that each developer is at least somewhat familiar with. On a side note, I would love if the default was for these code reviews to be recorded. That way 2 years later when I am asked to modify some module that no one has touched in that span, I could at least watch the code review and gleem something about how/why this was architect-ed the way it was. | | |
| ▲ | lokar 4 days ago | parent [-] | | IMO, a lot of what I think you are getting at should be worked out in design before work starts. |
| |
| ▲ | colinb 4 days ago | parent | prev [-] | | Fagan inspection has entered the room |
|