Hacker News Reader

Comment by meta_gunslinger | Hacker News Reader

Comprehensive end-to-end tests and CI can only attest to correctness, most engineers worth their salt won't review code only in regards to that aspect though.

View on HN

In the bad old days before auto-formatters and linters, PRs were heavily used to enforce style guidelines. If we can enforce both style and correctness in our CI pipeline, what is actually left?

Code architecture and technical design. You can have a solution that works fine, but are too complex or will impede future changes. Maybe you have code that has already been solved or your variables’ name are too generic. Maybe your modules are messy and your data structures are not modeled well.

vibe check

If the correctness check was vibecoded there's a good chance it was cheated. So maybe that, on top of the, you know, code review (see the sibling comment).

While PRs may have been used to correct style, that shouldn't have been their only or even main purpose. That's on whoever was using it that way, not on the concept of reviews.

The functionally correct code could be rejected in PR for many reasons other than style:

1. Solution under-engineered/over-engineered. 2. Code is hard to read or comprehend. 3. Design/Archtecture lacking. 4. Principles decided upon by team not adhered to.

These are just some of the reasons I've rejected functionally correct code before.

To summarize, in any software engineering course you learn that there are other metrics used to evaluate code other than correctness (maintainability, readability, scalability, portability, efficiency etc.)

As said already: readability and maintainability of the code (closely related) are two most important values a code review can get you.

Reply on news.ycombinator.com