Live data from Hacker News

Code review can be better

tigerbeetle.com

111–120 of 253 posts

Re: Code review can be better

#111
post #102

Earlier quoted context omitted.

> and that isn't something I ever encountered in the wild (in any formal sense) Because in the software engineering world there is very little engineering involved. That being said, I also think that the industry is unwilling to accept the slowliness of the proper engineering process for various reasons, including non criticality of most software and the possibility to amend bugs and errors on the fly. Other engineer…

It still is engineering you only mistake design phase. Writing code is the design phase. You don’t need design phase for doing design. Will drop link to relevant video later.

This is the talk on real software engineering:

https://www.youtube.com/watch?v=RhdlBHHimeM

Re: Code review can be better

#112
I use CodeRabbit that helps, but it does not fix the two root issues. I run their free VS code plugin to review local commits first, which catches nits, generates summaries, and keeps me in my editor. The PR bot then adds structure so humans focus on design and invariants. Review state still lives in the forge, not in Git, and interdiffs still depend on history. If Git gets a stable Change-Id, storing review metadata in Git becomes realistic. Until then this is a pragmatic upgrade that reduces friction without changing the fundamental.

https://www.coderabbit.ai/ide

Re: Code review can be better

#113

Just taking a step back, it is SO COOL to me to be reading about stacked pull requests on HN. When we started graphite.dev years ago that was a workflow most developers had never heard of unless they had previously been at FB / Google. Fun to see how fast code review can change over 3-4yrs :)

Given the security incident that happened to CodeRabbit I’m a bit less enthusiastic about testing out new tools that have LLMs and my codebase under the same tool.

What can be a very nice experiment to try something new can easily become a security headache to deal with.

Re: Code review can be better

#114

Just taking a step back, it is SO COOL to me to be reading about stacked pull requests on HN. When we started graphite.dev years ago that was a workflow most developers had never heard of unless they had previously been at FB / Google. Fun to see how fast code review can change over 3-4yrs :)

Stacked pull requests seem to add a layer of complexity to solve a problem that should and can be avoided in the first place.

Frequent, small changes are really a good practice.

Then we have things like trunk-based development and continuous integration.

Re: Code review can be better

#115

> But modifying code under review turned out to be tricky. GitLab enables this - make the suggestion in-line which the original dev can either accept or decline.

Kind of. Don't you have to type the change into the browser? Which means your change might not even be syntactically correct. It would be far better if you could make the change locally then somehow and that straight to GitLab. Also how does it work with multiple commits? Which commit does it amend?

Re: Code review can be better

#116
post #102

Earlier quoted context omitted.

> and that isn't something I ever encountered in the wild (in any formal sense) Because in the software engineering world there is very little engineering involved. That being said, I also think that the industry is unwilling to accept the slowliness of the proper engineering process for various reasons, including non criticality of most software and the possibility to amend bugs and errors on the fly. Other engineer…

It still is engineering you only mistake design phase. Writing code is the design phase. You don’t need design phase for doing design. Will drop link to relevant video later.

I also read this series of blog posts recently where the author, Hillel Wayne, talked to several "traditional" engineers that had made the switch to software. He came to a similar conclusion and while I was previously on the fence of how much of what software developers do could be considered engineering, it convinced me that software engineer is a valid title and that what we do is engineering. First post here: https://www.hillelwayne.com/post/are-we-really-engineers/

Re: Code review can be better

#117
post #81

What 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…

> Not that I know how to fix that. You can't have everyone in the entire company spend time looking at every possible thing that might be developed in the near future. Or can you?

You don't need to. I've seen this generally work with some mix of the following:

1. Try to decouple systems so that it's less likely for someone in a part of the org to make changes that negatively impact someone in a more distant part of the org.

2. Some design review process: can be formal "you will write a design doc, it will be reviewed and formally approved in a design committee" if you care more about integrity and less about speed, or can be "write a quick RFC document and share it to the relevant team(s)".

3. Some group of people that have broad context on the system/code-base (usually more senior or tenured engineers). Again, can be formal: "here is the design review committee" or less formal: "run it by these set of folks who know there stuff". If done well, I'd say you can get pretty broad coverage from a group like this. Definitely not "everyone in the entire company". That group can also redirect or pull others in.

4. Accept that the process will be a bit lossy. Not just because you may miss a reviewer, but also, because sometimes once you start implementing the reality of implementation is different than what people expect. You can design the process for this by encouraging POC or draft implementations or spikes, and set expectations that not all code is expected to make it into production (any creative process includes drafts, rewrites, etc that may not be part of the final form, but help explore the best final form).

I've basically seen this work pretty well at company sizes from 5 engineers all the way up to thousands.

Re: Code review can be better

#118
post #102

Earlier quoted context omitted.

> and that isn't something I ever encountered in the wild (in any formal sense) Because in the software engineering world there is very little engineering involved. That being said, I also think that the industry is unwilling to accept the slowliness of the proper engineering process for various reasons, including non criticality of most software and the possibility to amend bugs and errors on the fly. Other engineer…

It still is engineering you only mistake design phase. Writing code is the design phase. You don’t need design phase for doing design. Will drop link to relevant video later.

> Writing code is the design phase.

Rich Hickey agrees it's a part of it, yes. https://www.youtube.com/watch?v=c5QF2HjHLSE

Re: Code review can be better

#119
Leave the comments in the commit messages and make many small commits! That way they don't change the actual source and they're specific for that version of the code.

Re: Code review can be better

#120

Just taking a step back, it is SO COOL to me to be reading about stacked pull requests on HN. When we started graphite.dev years ago that was a workflow most developers had never heard of unless they had previously been at FB / Google. Fun to see how fast code review can change over 3-4yrs :)

Stacked pull requests seem to add a layer of complexity to solve a problem that should and can be avoided in the first place. Frequent, small changes are really a good practice. Then we have things like trunk-based development and continuous integration.

Stacked PRs allow me to post frequent, small changes without waiting for a review between each one.
Post reply on HN