3 ms·

I went ~25yrs into my coding career without code review and without tests. I wrote games, several AAA games, play testers found bugs, but otherwise there was no

by gernb 4y ago

I went ~25yrs into my coding career without code review and without tests. I wrote games, several AAA games, play testers found bugs, but otherwise there was no code based tests and no code review, none, zero, zilch. From 1983 to around 2008. Now I use tests and code review and I like them. I like that reviewers catch my bugs. I like that they suggest better solutions I didn't think of. I like that they tell me about functions I could use that make the code similar. I like that they help explain the code. But, I can't entirely dismiss that I and the teams I worked on shipped a ton of products without any code review and that my velocity feels much slower than previously. It's possible that slowness is from the projects being way more complex, the teams being much larger, and that code review is actually a perf multiplier over all for these larger more complex things I'm working. But, waiting for and/or doing reviews certainly "feels" slower than I used to be.

kafrofrite 4y ago
> I like that they suggest better solutions I didn't think of. Although I don't write code full-time, when I do this is the part I enjoy more. People reviewing my code and coming up with better solutions on that same problem amazes me.
nicoburns 4y ago
For a game that has a single release code review seems like it would be less important. Where I've found it really comes into it's own is keeping a codebase maintainable over several years.
imiric 4y ago
Sure, the programming landscape was much different 30 years ago. We also didn't rely on code versioning, CI/CD, testing, and many other practices we consider standard today. The industry has since evolved to make development better and more reliable, at the expense of relative speed or simplicity. In return, we gained the ability to minimize the chances of shipping broken code that is only caught by manual testers, or, worse, actual users. Game development particularly went through radical changes. I don't have much experience here, but I can imagine the stress of needing to ship bug-free code on actual physical media, and the excruciatingly slow dev-QA-release cycle. We're spoiled these days to have automated tests, peer review, and OTA updates, where bugs can be found, fixed, and bugfixes shipped much quicker than if we were not doing this. So I agree that all of these processes add a mental and time overhead, but I wouldn't go back to working without them for anything.
epage 4y ago
While I think code reviews are great, I think the ideal state is doing them asynchronously post-merge. Enforced pre-merge reviews are a way to hold the change hostage to force a certain quality bar. If there is the experience and trust on the team, you don't need to hold each other hostage. You know the author won't make too much of a mess, you know someone will come around to doing the review soon, getting the benefits of code review, and you know the author will be responsive to feedback and not just dismiss them because they moved on. Even when you do have pre-merge reviews, some times the culture and tooling can make things slower than they need to be. I saw this at one company: - Review system sent so many notifications, people ignored them - It was unclear if a change was relevant for you to review - When there is communal ownershipg no one owns it. The code review system needs to assign the reviewer from among the team - it was unclear when you owned reviewing something - slow CI and not having auto-merge makes people lose track of things. As a last resort consider a two stage CI, fast build of a subset for PR updates, full build on merge. - people should treat CI as another reviewer and not block on it

by gernb 4y ago

kafrofrite 4y ago

> I like that they suggest better solutions I didn't think of. Although I don't write code full-time, when I do this is the part I enjoy more. People reviewing my code and coming up with better solutions on that same problem amazes me.

nicoburns 4y ago

For a game that has a single release code review seems like it would be less important. Where I've found it really comes into it's own is keeping a codebase maintainable over several years.

imiric 4y ago

Sure, the programming landscape was much different 30 years ago. We also didn't rely on code versioning, CI/CD, testing, and many other practices we consider standard today. The industry has since evolved to make development better and more reliable, at the expense of relative speed or simplicity. In return, we gained the ability to minimize the chances of shipping broken code that is only caught by manual testers, or, worse, actual users. Game development particularly went through radical changes. I don't have much experience here, but I can imagine the stress of needing to ship bug-free code on actual physical media, and the excruciatingly slow dev-QA-release cycle. We're spoiled these days to have automated tests, peer review, and OTA updates, where bugs can be found, fixed, and bugfixes shipped much quicker than if we were not doing this. So I agree that all of these processes add a mental and time overhead, but I wouldn't go back to working without them for anything.

epage 4y ago

While I think code reviews are great, I think the ideal state is doing them asynchronously post-merge. Enforced pre-merge reviews are a way to hold the change hostage to force a certain quality bar. If there is the experience and trust on the team, you don't need to hold each other hostage. You know the author won't make too much of a mess, you know someone will come around to doing the review soon, getting the benefits of code review, and you know the author will be responsive to feedback and not just dismiss them because they moved on. Even when you do have pre-merge reviews, some times the culture and tooling can make things slower than they need to be. I saw this at one company: - Review system sent so many notifications, people ignored them - It was unclear if a change was relevant for you to review - When there is communal ownershipg no one owns it. The code review system needs to assign the reviewer from among the team - it was unclear when you owned reviewing something - slow CI and not having auto-merge makes people lose track of things. As a last resort consider a two stage CI, fast build of a subset for PR updates, full build on merge. - people should treat CI as another reviewer and not block on it