Live data from Hacker News

Code review can be better

tigerbeetle.com

241–250 of 253 posts

Re: Code review can be better

#241

Earlier quoted context omitted.

> You need a high level design up-front but it should not be cast in stone. Yes, you need a design that precedes code. > Writing code and iterating is how you learn and get to a good, working design. You are confusing waterfall-y "big design upfront" with having a design. It isn't. This isn't even the case in hard engineering fields such as aerospace where prototypes are used to iterate over design. In software engin…

The difference I see is that in other fields, part of your design process is thinking through the logical details of the thing. Essentially, doing some math. In software, the logical details are the finished product. The math is what you're trying to make. If you've actually thought through all of the details, you have written the software (if only in your head). If you haven't thought through all of the details and…

> The difference I see is that in other fields, part of your design process is thinking through the logical details of the thing. Essentially, doing some math.

The "some math" is used in engineering fields in things like preliminary design, sizing, verification&validation, etc. To a lesser degree, "some math" can be used in the design stages of software development projects. For example, estimating the impact of micro services tax in total response times to verify if doing synchronous calls can work vs doing polling/messaging. Another example is estimating max throughput per service based on what data features in a response and how infrastructure is scaled. This is the kind of things that you do way before touching code to determine if the expected impact of going with a particular architecture vs another that mitigates issues.

> In software, the logical details are the finished product. The math is what you're trying to make.

You're confused. The design stage precedes writing any code, let alone the finished product. Any remotely complex work, specially if it involves architecture changes, is preceded by a design stage where alternatives are weighed and validated, and tradeoffs are evaluated.

To further drive the point home, in professional settings you also have design reviews for things like security and data protection. Some companies even establish guidelines such as data classification processes and comparative design to facilitate these reviews.

> If you've actually thought through all of the details, you have written the software (if only in your head). If you haven't thought through all of the details and only figured out a high level design, you've still written some software (essentially, stubbing out some functionality, or leaving it as a dependency to be provided. However you want to think of it).

You're confusing having a design stage with having a big design upfront. This is wrong.

The purpose of the design stage is to get the necessary and sufficient aspects right from the start, before resources are invested (and wasted) in producing something that meets requirements. No one cares what classes or indentation style you use to implement something. The ultimate goal is to ensure the thing is possible to deliver, what it actually does and how it does it, and if it is safe enough to use. You start writing code to fill in the details.

Re: Code review can be better

#242
post #223

Earlier quoted context omitted.

Like in Portugal, and in many countries yes, usually having Software Engineer on legally bound contracts implies taking the final examination. However, already by having been through the degree there is a whole set of skills that one would not have gotten otherwise. Assuming that they actually did it the right way, and not getting through it with minimal effort.

> However, already by having been through the degree there is a whole set of skills that one would not have gotten otherwise. Of course you would/could. 1) a degree doesn't imply you've built any specific skills or retained any information, just that you passed a set of exams. I've met a huge bunch of people from important universities that clearly studied just to pass exams with good degrees, but where absolutely cr…

Except most people that call themselves Engineers without a degree aren't Leonardo Da Vinci, or any of the others listed by you.

Exceptional individuals that made an impact to mankind, regardless of which kind.

There are schools for special developed kids with advanced cognitive skills for a reason.

Also there is a huge difference between being immersed in an engineering degree between 3 to 5 years almost every single day with compulsory assignments, depending on the country, and occasionally watch a couple of videos, or read one or two books.

This all without getting into the soft and ethical skills that engineering degrees also require.

Re: Code review can be better

#243
post #236

Earlier quoted context omitted.

In fact, every definition of engineering used by professionals includes explicitly taking responsibility, including in law in many countries.

I will add this link once more. This is the talk on real software engineering: https://www.youtube.com/watch?v=RhdlBHHimeM

I think maybe you should watch this video, as it speaks only about software development and not engineering in any meaningful aspect.

Re: Code review can be better

#244
post #65
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.

Stash was not an acquisition. Stash was built from the ground up inside Atlassian during its golden age, by a bunch of engineers who really cared about performance. Though it helped that they didn't have Jira's 'problem' of having 8 figures of revenue hanging off a terrible database schema designed a decade ago.

You might be thinking of Fisheye/Crucible, which were acquisitions, and suffered the traditional fate of being sidelined.

(You are 100% correct that Stash/Bitbucket Server has also been sidelined, but that has everything to do with their cloud SaaS model generating more revenue than selling self-hosted licenses. The last time I used it circa 2024, it was still way faster than Bitbucket Cloud though.)

Source: worked at Atlassian for a long time but left a few years ago.

Re: Code review can be better

#245

Here's an alternative I've wondered about: Instead of one person writing code, and another reviewing it - instead you have one person write the first pass and then have another person adjust it and merge it in. And vice-versa; the roles rotate. Anyone tried something like this? How did it go?

Great idea, if you're fine with development time to take twice as long.

If you are used to PRs taking days (or over a week!) to merge, then I've found it's way faster to get someone to sit down and walk them through the code - which is not miles different from what's being proposed here.

Re: Code review can be better

#246
post #192

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…

Summoning Hillel Wayne.... https://www.hillelwayne.com/post/are-we-really-engineers/

He's spot on.

Re: Code review can be better

#247

Earlier quoted context omitted.

The difference I see is that in other fields, part of your design process is thinking through the logical details of the thing. Essentially, doing some math. In software, the logical details are the finished product. The math is what you're trying to make. If you've actually thought through all of the details, you have written the software (if only in your head). If you haven't thought through all of the details and…

> The difference I see is that in other fields, part of your design process is thinking through the logical details of the thing. Essentially, doing some math. The "some math" is used in engineering fields in things like preliminary design, sizing, verification&validation, etc. To a lesser degree, "some math" can be used in the design stages of software development projects. For example, estimating the impact of micr…

Design reviews don't mean you don't write code first. We have a change that we're about to put through security review right now to special case TLS handling when talking to some older clients, but before we put a proposal forward, I validated that the core idea will actually work and that we can heavily restrict the scope of what the change applies to by writing the code and tests to do it. Meetings and asking multiple people to read through and provide feedback on documents very quickly racks up hours. Best to spend an hour or two first validating that what you're proposing isn't nonsense. This lets us be very specific about what the problem is, what the proposed solution is, and what the risks are.

With data classification, you're going to need to think through what data you are using and what you want to do with it. i.e. write a program.

I didn't claim class structure or indentation matters. I'm saying that assuming you are discussing some sort of algorithm or functionality, a formal language is a perfectly fine thing to use for thinking about the problem and writing down your ideas. Writing "what it actually does and how it does it" is just programming. If you write your ideas in a language like Scala, they can easily be more concise (so easier to review) than they would be in English, and you get a compiler helping you think through things.

Re: Code review can be better

#248
post #196

Earlier quoted context omitted.

Is there something that does this? I've played with git town which is great for what it is. But at $DAYJOB we are now all on graphite and that stacking is super neat. The web part is frustratingly slow, but they got stacking working really well.

we have a big effort in the works to improve web perf! where specifically are you seeing slowness in the app — what flows, what pages, etc?

Super glad to hear!

The worst offender is a slack notification[0] deep link into a PR I need to review.

It loads in stages, and the time from click to first diff is often so frustratingly long that I end up copying the PR ID and going to GitHub instead.

Sometimes I give up while Graphite is still loading and use the shortcut C-G to go to GitHub.

The second issue might be the landing page. I love what it shows compared to GitHub, but it's often slow to display loading blocks for things that haven’t even changed. Reloads are usually fast after that — until sometime later, maybe a day, when it slows down again.

I don't know why it feels worse than Linear, even though there are clearly many similarities in how it's supposed to load.

The guest instance isn’t so much about loading speed, but usage speed.

When I submit a stack of PRs, I get a nice carousel to fill in PR titles/descriptions and choose where to publish each PR. What’s missing for me there is access to files and diffs, so I can re-review before publishing. I often end up closing it and going back to the PR list instead.

[0] Thank God for those! You've made them much more useful than GitHub's. Also, the landing page is far more helpful in terms of what’s displayed.

Re: Code review can be better

#249
post #71
post #51

Earlier quoted context omitted.

Part of the problem is that businesses treat reviewing as a second class citizen. It's not "actual work" so shouldn't be given preference, which excuses the LGTM style reviews. I've been out of the industry for a while but I felt this way years ago. As long as everybody on the team has coding tasks, their review tasks will be deprioritized. I think the solution is to make Code Reviewer a job and hire and pay for it,…

I don't see a lot of value in generic code reviewers. I want the reviewers to be actively engaged in writing somewhat related code themselves, otherwise the value of their opinions will decline over time. As for prioritization... isn't it enough knowing that other people are blocked on your review? That's what incentivizes me to get to the reviews quickly. I guess it's always going to depend a lot on your coworkers a…

Someone brought up the point that more people will be doing more reviewing as more code is written by AI.

If your job description is reviewing the codebase and every change that goes into it, you will be actively engaged. Whoever the most fervent auditor of new packages/libraries is on the team, they're probably de facto doing this role. Whoever has the deepest knowledge actually, just let them observe/edit.

Re: Code review can be better

#250
post #244
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.

Stash was not an acquisition. Stash was built from the ground up inside Atlassian during its golden age, by a bunch of engineers who really cared about performance. Though it helped that they didn't have Jira's 'problem' of having 8 figures of revenue hanging off a terrible database schema designed a decade ago. You might be thinking of Fisheye/Crucible, which were acquisitions, and suffered the traditional fate of b…

Yeah I think I was remembering things backwards - since they put Stash under the Bitbucket organisation and branding it looked as if Bitbucket was their own product and Stash the outside acquisition, but it was actually the other way around.
Post reply on HN