Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

While I don't agree with Charles' (clearly nuts) distaste for sparkling water, I'm happy to see people finally talking about this issue publicly.

I've suffered through this at several companies, down to the level of sometimes spending like 3x the time it took to implement the actual feature on answering and 'fixing' pedantic stylistic nitpicks during code reviews. While having a homogenous style is important, I'm solidly in the camp of "if you want to give me style nitpicks, just change them yourself, tell me, and save us both some time". This extends to like 80% of feedback I see in code reviews.

Be weird and do stuff. https://www.youtube.com/shorts/DjvVN4Vp_r0



Have a formatter. There are plently out there that can be part of the compile/build/commit flow, to the point of failing pipelines if the files you changed not match the style.

Let people know if you have a requirement on layout and enforce it. Code review is far too late.


Where I work the formatter is the final arbiter of formatting code. If you don't like it, good luck justifying a change and you had better have really thought about it and why your change is good, because thousands of engineers have tried and been defeated for one reason or another :)


> Where I work the formatter is the final arbiter of formatting code.

And I'm fine with that.

Consistency is key for readability, code is read more than written, legacy code is code that people can't read anymore and want to start again, and most of the changes people want to make aren't standard making it harder for new team members to read the code.

Install Black, Prettier, Spotless, and move on.


I like it too. It is reliable, predictable. Removes a common source of opinionated friction.


> style nitpicks

Other commenter point at auto-formating, but I think companies not using any are pretty rare, hitting that very issue in several companies is highly improbable.

We're probably down to function/variable naming, ternal operators, return types, this kind of thing ?

Someone who can't bother looking around and adapt their style to the surrounding code sounds like a problem to me. What is seen as pendantic and nitpicky can have pretty large impact on readability and go against others' assumptions.


Even more than that, it means the developer has spent no time understanding the surrounding code, and thus is likely adding debt/CRUFT/risk.


This has no relation with collaboration. Being nitpicky about a PR is wrong, you should have a linter or enforce the culture of not being nitpicky about code.

This is not a "culture of collaboration" by any means.


Every delivered feature is a liability not an asset.

If you don’t believe me, consider two products that make customers equally happy and one has half as many moving parts. Which one is more profitable to maintain? The one with less shit. Because the rest is just more liability.

So if I deliver a feature all by myself quickly and move on to something else, I’m digging a bigger hole and faster than the wisdom arrives to change directions.

But most importantly, none of the rest of you fuckers know what I built or why, except what you gleaned from standup or whatever docs I wrote in place of collaboration. And docs written without collaboration are usually hot garbage.

So what do you all do when I’m hiking in the woods and the cluster is on fire? Sure would be nice if I collaborated on that functionality wouldn’t it? Then you’d have two other people to ask.


> Every delivered feature is a liability not an asset.

> If you don’t believe me, consider two products that make customers equally happy and one has half as many moving parts. Which one is more profitable to maintain? The one with less shit. Because the rest is just more liability.

You're mixing up the feature and the moving parts.

A feature is an asset. Moving parts (or code) are the liability. They are not the same.

Sometimes you can even improve the product by removing code, or refactoring things so you have less or clearer code while at the same time improving the product. Maybe you unify some UI, so more functionality is available in more places, while also cleaning up the code, maybe reducing line count.

> So what do you all do when I’m hiking in the woods and the cluster is on fire? Sure would be nice if I collaborated on that functionality wouldn’t it? Then you’d have two other people to ask.

It's a fair point, but depends on the scale I would say. Some smaller things can be easily understood (but maybe that's not the stuff that causes the cluster to be on fire). Also, IMO at least one person should have reviewed the stuff, and therefore be at least a little bit knowledgeable about what's going on.


> You're mixing up the feature and the moving parts.

No, you are, and this is why so many of us are shit at UX.

The ability to accomplish a task with software is an asset. Not every application takes the same number of interactions to finish a task, and each interaction is a feature.

Features in neither waterfall, scrum, nor Kanban directly correlate with the ability of the user to accomplish something meaningful. They are units of measure of the developer accomplishing something meaningful, but the meaning is assumed, not real.


> Every delivered feature is a liability not an asset.

> If you don’t believe me, consider two products that make customers equally happy and one has half as many moving parts. Which one is more profitable to maintain?

Wrong analogy, use the word feature in both emphasized terms. Consider two products and one has half as many features, which is more profitable to maintain? Well, it depends whether people are paying for the other half of the feature set or not, as oftentimes people will pay for more features than fewer.


It’s a perfectly fine analogy if you can get over your own ego enough to realize your customers don’t want to hear about how very clever you are, they just want to get shit done and move on to four other tasks.

They don’t care about us. They don’t. They just want to do what their boss asked them to do or kill the bad guy to get the treasure, and we are often enough as much in the way as we are facilitating that.


This sounds like a non sequitur to me, when did I ever say I disagreed with the fact that "they just want to get shit done?" I am not sure what your comment has to do with the part about misconstruing features for moving parts, for those are two independent things, and still more generally, like I said, people do pay for software that has more features than fewer.


Can't help but think this is human nature.

Ask anyone in a relationship if their partner complains about unimportant stuff. With experience and practice you can get past this stuff.

so... experienced reviewers.


> their partner complains about unimportant stuff.

I can't know how you meant it, but if your partner if complaining about something 5 times a day, you might want to rethink if it's really unimportant, and understand why they care instead of "get past this stuff".

That reminds me of the guys getting dumped after 10 years of marriage complaining they genuinely have no idea why their partner left them.


unimportant stuff = syntax errors

communication = both parties learning to communicate in effective ways

"tab stops might seem unimportant, but I can read and review your code easier"

"Ok I see your concern and will set up my editor"

"toilet seat seems unimportant, but if I fall in in the middle of the night, it will disturb both of us"

:)


Who decides what is a nitpick and what is essential to keeping code clean enough that others can do their job?


amen. i don't really care about style/format. my opinion is that if you care you should automate it.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: