Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Code review can be better

tigerbeetle.com

Post reply on HN
Print view View original

81–90 of 253 posts

Re: Code review can be better

#81

Post by 3036e4 »

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 been done at all.

It always seems as if the code review is the only time when all stakeholders really gets involved and starts thinking about a change. There may be some discussion earlier on in a jira ticket or meeting, and with some luck someone even wrote a design spec, but there will still often be someone from a different team or distant part of the organization that only hears about the change when they see the code review. This includes me. I often only notice that some other team implemented something stupid because I suddenly get a notification that someone posted a code review for some part of the code that I watch for changes.

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? I don't know. That doesn't seem to ever happen anyway. At university in the 1990's in a course about development processes there wasn't only code reviews but also design reviews, and that isn't something I ever encountered in the wild (in any formal sense) but I don't know if even a design review process would be able to catch all the things you would want to catch BEFORE starting to implement something.

Re: Code review can be better

#82

Post by aitchnyu »

koolba wrote: post #14

> When I review code, I like to pull the source branch locally. Then I soft-reset the code to mere base, so that the code looks as if it was written by me. This is eerily similar to how I review large changes that do not have a clear set of commits. The real problem is working with people that don’t realize that if you don’t break work down into small self contained units, everybody else is going to have to do it ind…

We were a team with highly parallizable data science tasks, we checked out 3 copies of the repo, one for our branch, two for branches where we are the reviewer.

Re: Code review can be better

#83

Post by dotancohen »

lmm wrote: post #65
echelon wrote: post #53

Earlier quoted context omitted.

> The biggest grip I have with Github is the app is painfully slow. And by slow, I mean browser tab might freeze level slow. Javascript at scale combined with teams that have to move fast and ship features is a recipe for this. At least it's not Atlassian.

Stash (now BitBucket Server) had the best code review going, head and shoulders above GitHub to the point I thought GitHub would obviously adopt their approach. But I imagine Atlassian has now made it slow and useless like they do with all their products and acquisitions.

Bit Bucket had a git-related tool called Stash? I love Bit Bucket, but I'm glad I did not know about that.

Re: Code review can be better

#85

Post by aitchnyu »

Tangential, long ago I wanted to use a repo-backed (IIRC tied to Mercurial) backend for issues. They were also flat files. We put too many plugins into Redmine and it died frequently.

Re: Code review can be better

#86

Post by vinnymac »

tomasreimers wrote: post #16

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 :)

Just signed up, looks like everything I ever wanted

Re: Code review can be better

#87

Post by epolanski »

3036e4 wrote: 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…

> 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 engineering fields enjoy no such luxuries, the bridge either holds the train or it doesn't, you either nailed the manufacturing plant or there's little room for fixing, the plane's engine either works or not

Different stakes and patching opportunities lend to different practices.

Re: Code review can be better

#88

Post by 3036e4 »

jacobegold wrote: post #22

It's so cool that Git is considering first class change IDs!! That's huge! This sounds similar to what we had at Facebook to track revisions in Phabricator diffs. Curious if anyone knows the best place to read about this?

The fundamental problem is that git doesn't track branches in any sane way. Maybe it would be better to fix that? Fossil remembers what branch a commit was committed on, so the task branch itself is a change ID. That might be tricky to solve while also allowing git commands to mess with history of course. Fossil doesn't have that problem.

Re: Code review can be better

#89

Post by mgaunard »

jbmsf wrote: post #61

Recently, I've been wondering about the point of code review as a whole. When I started my career, no one did code review. I'm old. At some point, my first company grew; we hired new people and started to offshore. Suddenly, you couldn't rely on developers having good judgement... or at least being responsible for fixing their own mess. Code review was a tool I discovered and made mandatory. A few years later, everyo…

The way I like to do it, is that some projects may have an owner (can only be a single person).

The owner is allowed to make changes without review.

Re: Code review can be better

#90

Post by mgaunard »

Feeble wrote: post #68
jbmsf wrote: post #61

Recently, I've been wondering about the point of code review as a whole. When I started my career, no one did code review. I'm old. At some point, my first company grew; we hired new people and started to offshore. Suddenly, you couldn't rely on developers having good judgement... or at least being responsible for fixing their own mess. Code review was a tool I discovered and made mandatory. A few years later, everyo…

I am very much in the same position right now. My dev team has introduced mandatory code reviews for every change and I can see their output plummeting. It also seems that most code reviews done are mostly syntax and code format related - noone actually seems to run the code or look at the actual logic if it makes sense. I think its easy to add processes under the good intention of "making the code more robust and cl…

You need to validate things like syntax upfront so that such things don't make it to review to begin with.

I'm not a fan of automatic syntax formatting but you can have some degree of pre-commit checks.

Post reply on HN