Live data from Hacker News

Code review can be better

tigerbeetle.com

91–100 of 253 posts

Re: Code review can be better

#93
Author has not tried IDE plugin for GitHub / Bitbucket reviews!? We review code directly in IntelliJ with full support for commenting, navigation, even merging without leaving the IDE. Solves all problems the author has.

Re: Code review can be better

#94
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…

Seems like your organization is lacking structure and communication.

Where I work, the structure is such that most parts of the codebase have a team that is responsible for it and does the vast majority of changes there. If any "outsider" plans a change, they come talk to the team and coordinates.

And we also have strong intra-team communication. It's clear who is working on what and we have design reviews to agree on the "how" within the team.

It's rare that what you describe happens. 95% of the code reviews I do are without comments or only with minor suggestions for improvement. Mainly because we have developed a culture of talking to each other about major things beforehand and writing the code is really just the last step in the process. We also have developed a somewhat consistent style within the teams. Not necessarily across the teams, but that's ok.

TL;DR: It's certainly possible to do things better that what you are experiencing. It's a matter of structure, communication and culture.

Re: Code review can be better

#95
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…

Where I work we tend to write RFCs for fundamental design decisions. Deciding what counts as a "fundamental design decision" is sometimes self-moderated in the moment but we also account for it when making long term plans. For example when initially creating epics in Jira we might find it hard to flesh out as we don't really know how we're going to approach it, so we just start it off with a task to write an RFC.

These can be written either for just our team or for the eyes of all other software teams. In the latter case we put these forward as RFCs for discussion in a fortnightly meeting, which is announced well in advance so people can read them, leave comments beforehand, and only need to attend the meeting if there's an RFC of interest to them up for discussion.

This has gone pretty well for us! It can feel like a pain to write some of these, and at times I think we overuse them somewhat, but I much prefer our approach to any other place I've worked where we didn't have any sort of collaborative design process in place at all.

Re: Code review can be better

#96

I find the idea of using git for code reviews directly quite compelling. Working with the change locally as you were the one who made it is very convenient, considering the clunky read-only web UI. I didn't get why stick with the requirement that review is a single commit? To keep git-review implementation simple? I wonder if approach where every reviewer commits their comments/fixes to the PR branch directly would w…

> I didn't get why stick with the requirement that review is a single commit

Yeah that is pretty weird. If 5 people review my code, do they all mangle the same review commit? We don't do that with code either, feels like it's defeating the point.

Review would need to be commits on top of the reviewed commit. If there are 5 reviews of the same commit, then they all branch out from that commit. And to address them, there is another commit which also lives besides them. Each commit change process becomes a branch with stacked commits beinf branches chained on top of one another. Each of the commits in those chained branches then has comment commits attached. Those comment commits could even form chains if a discussion is happening. Then when everybody is happy, each branch gets squashed into a single commit and those then get rebased on the main branch.

You likely want to make new commits for that though to preserve the discussions for a while. And that's the crux: That data lives outside the main branch, but needs to live somewhere.

Re: Code review can be better

#97
Very nice to read.

Sourcehut is missing in the list; it’s built on the classical concept of sending patches / issues / bugs / discussion threads via email and it integrates this concept into its mailing lists and ci solution that also sends back the ci status / log via email.

Drew Devault published helpful resources for sending and reviewing patches via email on git-send-email.io and git-am.io

Re: Code review can be better

#98

The biggest grip I have with Github is the app is painfully slow. And by slow, I mean browser tab might freeze level slow. Shockingly, the best code review tool I've ever used was Azure DevOps.

What did you like so much about DevOps? I use it every day and don't have any issues with the review system, but to me it's very similar to github. If anything, I miss being able to suggest changes and have people click a button to integrate them as commits.

I've used this 'suggestion' workflow in azure devops. https://devblogs.microsoft.com/devops/introducing-the-new-pu...

Re: Code review can be better

#99
post #68
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…

The value of having more people actually see the code will be there even if it’s just an unnecessary syntax nitpick.

Re: Code review can be better

#100
post #65

Earlier quoted context omitted.

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.

There was a locally-hosted Git server platform called Stash. Atlassian bought it, rebranded it as "BitBucket Server" (positioned similarly to GitHub Enterprise or self-hosted GitLab) and gradually made it look and feel like BitBucket (the cloud product), even though they're actually completely separate codebases (or at least used to be).
Post reply on HN