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

171–180 of 253 posts

Re: Code review can be better

#171

Post by hgomersall »

closewith wrote: post #168
hgomersall wrote: post #166

Earlier quoted context omitted.

There are plenty of engineering of physical things where nobody has or takes responsibility. Equally, there's plenty of examples of software where careful processes are in place to demonstrate exactly the responsibilities you discuss.

> There are plenty of engineering of physical things where nobody has or takes responsibility. That is by definition not engineering. > Equally, there's plenty of examples of software where careful processes are in place to demonstrate exactly the responsibilities you discuss. Software engineering of course exists, but 99%+ of software is not engineered.

Ah, you've defined yourself to be right. Nice move.

I'm not sure the generally accepted definition of engineering makes any reference to taking responsibility: https://dictionary.cambridge.org/dictionary/english/engineer...

Re: Code review can be better

#172

Post by ncruces »

mytailorisrich wrote: post #130
radlad wrote: post #120

Earlier quoted context omitted.

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

Well, you don't need stacked PRs for that... I think stacked PRs are a symptoms of the issues the underlying workflow (feature branches with blocking reviews) has.

“What you'll do next and in what way” is often an important tool to put the small changes into context.

Stacked pull requests can be an important tool to enable “frequent, small changes” IMO.

Sure, I can use a single pull request and a branch on top of that, but then it's harder for others to leave notes on the future, WIP, steps.

A common situation is that during code review I create a few alternative WIP changes to communicate to a reviewer how I might resolve a comment; they can do the same, and share it with me. Discussion can fork to those change sets.

Gerrit is much closer to my desired workflow than GitHub PRs.

Re: Code review can be better

#173

Post by pjmlp »

epolanski wrote: post #87
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 engineer…

There is plenty on large scale enterprise projects, but than whole that stuff is looked down by "real developers".

Also in many countries, to one call themselves Software Engineer, they actually have to hold a proper degree, from a certified university or professional college, validated by the countrie's engineering order.

Because naturally 5 year (or 3 per country) degree in Software Engineering is the same a six weeks bootcamp.

Re: Code review can be better

#174

Post by mytailorisrich »

ncruces wrote: post #172
mytailorisrich wrote: post #130

Earlier quoted context omitted.

Well, you don't need stacked PRs for that... I think stacked PRs are a symptoms of the issues the underlying workflow (feature branches with blocking reviews) has.

“What you'll do next and in what way” is often an important tool to put the small changes into context. Stacked pull requests can be an important tool to enable “frequent, small changes” IMO. Sure, I can use a single pull request and a branch on top of that, but then it's harder for others to leave notes on the future, WIP, steps. A common situation is that during code review I create a few alternative WIP changes to…

From a continuous integration perspective my understanding is that stacked pulled requests do not make change more frequent if we define a "change" as being committed on the master branch. They only split the feature branch into smaller chunks. On the other hand, I do take your point about context over a number of consecutive changes.

But, to me, "creating a few alternative WIP changes to communicate to a reviewer" indicates an issue with code reviews. I don't think code reviews are the time to propose alternative implementations, even if you have a "better" idea unless the code under review is broken.

Re: Code review can be better

#175

Post by wolvesechoes »

gabrielpoca118 wrote: post #155
wolvesechoes wrote: post #127

Earlier quoted context omitted.

Personally I don't need to talk with "traditional" engineers to have an opinion there, as I am mechanical engineer that currently deals mostly with software, but still in the context of "traditional" engineering (models and simulation, controls design). Definitely making software can be engineering , most of the time it is not, not because of the nature of software, but the characteristics of the industry and culture…

But what about other engineering fields? From what I understand, if you compare it to chemical engineering, you have many more similarities, because you’re doing Hypothesis -> Experiment -> Analyze -> Refine -> Repeat, which seems very similar to what we do in software

Mechanical engineering also uses prototypes, iteration, lab testing etc. Building architects build multiple models before the first shovel is put into the ground.

Software is clearly different than "hardware", but it doesn't mean that other industries do not use experiment and iteration.

Re: Code review can be better

#176

Post by im_lince »

mike_hearn wrote: post #152

I've used a more hard-core version of this in my own company and have been meaning to write a blog post about it for years. For now an HN comment will suffice. Here's my version, the rationale and the findings. WORKFLOW Every repository is personal and reviewer merges, kernel style. Merging is taking ownership: the reviewer merges into their own tree when they are happy and not before. By implication there is always…

This hits home. I’ve run into the same pain with conventional web-based review tools: slow, nitpicky, and nobody really “owns” the merge. Your kernel-style approach makes a ton of sense — putting the reviewer on the hook changes the dynamic completely. And pushing FIXMEs straight into the branch instead of playing comment-ping-pong? That’s a huge quality-of-life win.

We’ve gone a slightly different route at my team. Instead of reinventing the workflow around Gitolite/IntelliJ, we layered in LiveReview(https://hexmos.com/livereview/). It’s not as hardcore, but it gives us a similar payoff: reviewers spend less time on drudge work because LiveReview auto-catches a ton of the small stuff (we’re seeing ~40% fewer prod bugs). That leaves humans free to focus on the bigger design and ownership questions — the stuff machines can’t solve.

Different tools, same philosophy: make review faster, saner, and more about code quality than bureaucracy.

Re: Code review can be better

#177

Post by spike021 »

ivanjermakov wrote: post #153
spike021 wrote: post #41

Earlier quoted context omitted.

is github's PR considered read-only? i've had team members edit a correction as a "suggestion" comment and i can approve it to be added as a commit on my branch.

By read-only I meant that you can't fully interact with the code: run/debug it, use intellisense, etc.

Can't you just check out the branch of the repo locally into your ide? i'm still confused what limitation you are talking about.

Re: Code review can be better

#178

Post by [deleted] »

pjmlp wrote: post #173
epolanski wrote: post #87

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…

There is plenty on large scale enterprise projects, but than whole that stuff is looked down by "real developers". Also in many countries, to one call themselves Software Engineer, they actually have to hold a proper degree, from a certified university or professional college, validated by the countrie's engineering order. Because naturally 5 year (or 3 per country) degree in Software Engineering is the same a six we…

[deleted]

Re: Code review can be better

#179

Post by wolvesechoes »

hgomersall wrote: post #156
wolvesechoes wrote: post #127

Earlier quoted context omitted.

Personally I don't need to talk with "traditional" engineers to have an opinion there, as I am mechanical engineer that currently deals mostly with software, but still in the context of "traditional" engineering (models and simulation, controls design). Definitely making software can be engineering , most of the time it is not, not because of the nature of software, but the characteristics of the industry and culture…

Engineering is just about wielding tools to solve problems. You don't need to use formal methods to do engineering in general. Sometimes they're useful; sometimes they're required; often they just get in the way. In the context of software vs other sub-disciplines, the big difference is in the cost of iterating and validating. A bridge has very high iteration cost (generally, it must be right first time) and validati…

"In the context of software vs other sub-disciplines, the big difference is in the cost of iterating and validating."

People forget that software is used in those other disciplines. CFD, FEA, model-based design etc. help to verify ideas and design without building any physical prototype and burning money in the real lab.

You can do some strain and stress analysis on a virtual bridge to get a high degree of confidence that the real bridge will perform fine. Of course, then you need to validate it at all stages of development, and at the end perform final validation under weight.

The thing is that people building engines, cars, planes, sensors, PCBs and bridges actually do so, largely because they are required to do so. If you give them freedom to not do that, many of them will spare themselves such effort. And they understand the principles of things they are working on. No one requires any of that from someone that glued together few NPM packages with a huge JS front-end framework, and such person may not even know anything about how the HTTP works, how browser handles the DOM etc. It's like having a mechanical engineer that doesn't even understand basic principles of dynamics.

There are industries that deal with the software (i.e. controls design) that have much higher degree of quality assurance and more validation tools, including meaningful quantitative criteria, so it clearly is not a matter of software vs hardware.

Re: Code review can be better

#180

Post by im_lince »

kjgkjhfkjf wrote: post #35

If you want to remain relevant in the AI-enabled software engineering future, you MUST get very good at reviewing code that you did not write. AI can already write very good code. I have led teams of senior+ software engineers for many years. AI can write better code than most of them can at this point. Educational establishments MUST prioritize teaching code review skills, and other high-level leadership skills.

Totally agree with this. Code review is quickly becoming the most important skill for engineers in the AI era. Tools can generate solid code, but judgment, context, and maintainability come from humans. That’s exactly why we built LiveReview(https://hexmos.com/livereview/) — to help teams get better at reviewing and learning from code they didn’t write.
Post reply on HN