Comment by ozim | Hacker News Reader

Comment by ozim | Hacker News Reader

Parent Full thread

Style formatting should be automated on level of IDE or other CI tooling.

Discussing or fixing code style is huge waste of time and styling can be automated. Yes it will be broken in some edge cases but for me that is acceptable because my goal is to deliver the feature and not to make "perfectly aligned code". Aligning code by hand is waste of time, thinking about aligning code is waste of time. Formatting should be in configuration and you format whole file at once and never do stuff by hand.

Passing tests is also easy to automate when creating PR, CI should run tests and tell that to person creating PR - hey tests failed, fix it and then after they pass you can open PR, not a fellow developer because that is waste of time for everyone.

I did not write "tests should not be there" only "are all tests passing?" is not part of code review for me, it is something that is there before code review even begins.

So checking if there are proper tests and if tests make sense is part of code review.

Thanks, that better helped me to understand your comment. Additionally I was not aware if that was with pull-requests or more with regular code-reviews as part of the development process. That was an additional insight, thanks for sharing.

Nevertheless, things can be different, e.g. if CI passes, it merges already, no need to have a request for merge, which are often a blocker to keep a steady development flow. Naturally, this benefits from tests that are already run during development, if not driving the development.

And I didn't read you're against testing nor aligning white space and comments at all, more the localization. Actually the points you raise are looking important to me, because if alignment comes in late, this can cause a lot of erosion, which can hinder any review if not even provoke merge conflicts which are stopping the process quite early and require re-iteration.

Your approach should also work towards non-release blocking code reviews, a property I personally like, as I've seen teams struggle with code reviews, becoming more and more of a burden, even after practicing it has found its way. But that is only a subjective comment of mine, every project is different, which makes it an interesting topic for me.

Reply on news.ycombinator.com