Hacker News new | ask | show | jobs
by TranquilMarmot 253 days ago
When I first tried out Prettier back in the 2010s, I was immediately hooked on the idea of using an opinionated formatter. We had constant arguments about formatting, and our many JS codebases were a complete disaster as they had been worked on for many years by many people with nobody overseeing any of the formatting.

I was a zealot - I spent weeks going into every single one of our JS codebases, running Prettier on all of the files, setting up CI/CD rules to enforce it, and teaching everybody how to set up format-on-save in their editors. Some people whined about it ("I don't like the way it does curly braces! Waaah!") but I was persistent and unwavering.

All these years later, I'm elated that whenever I join a new company - engineering orgs almost always have all of this already set up, not just with JS/TS but almost every language out there. It makes getting up to speed on the actual content of the code so much faster.

If I join your engineering org and you aren't using enforced, automatic, opinionated formatting on every single line of code that gets written that's a huge smell.

3 comments

One of my main pet peeves of working with other developers is people breaking from conventions because "it is not good". Yes even if a convention is bad you should still follow it, unstandardized codebase is the main root of all evil in large codebases.

I try to avoid setting up conventions that can't be statically enforced (or at least enforced through tests/assertions). So yes, auto-formatters are a godsend because they statically enforce code-style.

It has become standard feature in most languages stacks these days because of Prettier and gofmt.

I've recently worked for a company that had no linter and no formatter. It was Java backend, but we have those tools too :) My first PR received 70 comments, 98% was nit-picking. Things like "Rename `MyDomainEntity source` to `MyDomainEntity myDomainEntity`" (it was a MapStruct mapper). I was also asked to remove a simple parent class that I used (in tests!) to reduce duplicated code, something like `MyNewFeatureBaseTest`, because Create, Get, Update, Patch, Delete test classes where sharing some functionalitites.

They fired me during the trial period after this PR.

Before this experience I was also "let's follow conventions even if they are bad", but now that I saw this argument applied so blindly against me, where I wasn't even allowed to use a common class to avoid duplicated code, I don't know, it just feels wrong.

3 weeks later and I'm still unsure if I was really wrong or just being gaslighted. I get conventions like "we split tests in create, get, etc. classes" or "the type of the incoming payload must be called `SomeEntityInput` and the output must be called `SomeEntityOutput`", but 70 comments? And you want me to duplicate code because that's what you did until now, "because conventions"?

I am sorry for your experience but I do agree with you, the conventions you were dealing with are bad. It is preferable to avoid conventions that can't be statically enforced specifically to avoid this kind of scenario you are describing.
+1.

I'm a huge fan of custom linter rules.

Instead of going through PRs and nit-picking stuff, I just create new custom linter rules that catch those team-specific or project-specific things.

This has the positive effect of making things way more welcoming for junior devs and newcomers. Nothing replaces getting the first PR with almost no PR comments thanks to a linter.

I hate that a lot of people read the assertion that "PR reviews are a form of creating an internal coding culture" and become human linters, only nit picking PRs for the power trip. This is not only annoying but also makes people ignore the more important parts of the review.

I even made it a rule in every team I managed: if something is bothering you constantly in PRs and it can be replaced by a custom linter rule, let it go and write a linter rule for that later. I only met two people in my career who opposed this, and that was 100% because it made their nit-picking redundant.

The difficulty that I've run into is that one person's nit is another person's essential change. My experience has led me believe that there are several different ways that developers approach reading code; resulting in developers placing different value on different style rules.

For example, when I am skimming code I primarily look at the code's "shape" and "flow". Where is there iteration? What code blocks are mutually exclusive? How does the code bail if there's an error? Etc. Therefore, things like indentation style and function decomposition are important to me because I need to be able to quickly see the overall layout of the code.

As far as I can tell, other developers read code using more of a "depth first" style where the overall layout isn't as important. As a result, they tend not to care much about things like indentation style or decomposition.

Which tools are you using that allow custom linter rules? I'd like to try this on some of my repos
In ESLint I use eslint-plugin-local-rules and have my rules in a .js file (or folder) in the repo.

For golang I just use go/ast directly and make a small tool.

eslint and biome allow custom lint rules and it is not that hard to write new ones yourself.
I often say that “nit” comments on prs are bad. It’s either important enough to make a linter rule, or not important enough to mention.
I wouldn't go that far - to me what's important is that nits must be non-blocking. I.e. the author is free to ignore them if they disagree, and a PR review with only nit comments should be an approval. If those things are true, it's fine to have gray area between what's linted and what's mentioned in PR review.
+1

For example, "nit: maybe call the function updateCreditCard instead of updateCard"

If you disagree and think your function name is better (or that the two are equally bad), then I'm happy to go along with what you've got. But maybe you didn't think of this name or maybe I've convinced you. Either it's a quick fix (and no re-review needed) or you just dismiss my comment.

100% agree! The pattern that I've found works well is:

Reviewing a PR with "Approval" status means that the comments are just suggestions.

Choosing "Comment" status means that the comments are optional but important enough that I want to make sure that you read them. If you come back and say "I read them but decided not to make any changes" then I'm happy to approve the PR.

Choosing "Request changes" means that the comments aren't just nits.

Sounds similar to my personal history of adopting StandardJS (https://github.com/standard/standard) as soon as it appeared, doing the same at the org I was at the time. Based on memory, seems StandardJS appeared before Prettier in the ecosystem?

It seems weird that StandardJS wasn't mentioned in this article, as surely the author must have known about it before starting Prettier.

Still to this day I just use StandardJS when starting a new project, not even sure what the difference against Prettier is, it's more or less the same as far as I can tell but maybe faster?

Standard always struck me as kind of scummy - it tries to present itself as something bigger than what it really is, which is a tiny CLI wrapper around one guy’s ESLint config. You have to scroll way down the FAQ section before ESLint even gets mentioned. The author is just squatting on the “standard” namespace and using it to blag his way into relevance. Think how much more good it would do if the sponsors of this project were actually supporting ESLint directly instead of this useless middleware.
this 100% is why it isn't mentioned. StandardJS is just a scam that nobody uses except new devs who get tricked by its pervasive marketing.
> format-on-save

A dystopian feature!

I'll do one better: In the recent teams I worked, we even had "organize imports on save".
I love format on save. I can type (or paste) ignoring whitespace, hit save, and everything pops right into place for me.

Also, stuff not auto formatting is usually immediate feedback that I didn’t match my parenthesis or have some other major syntax error.

Yes, some people do not see the light. Ever since starting to rely on prettier + format on save, I can just bang out code and focus more on content than formatting and let it handle everything for me. Speed multiplier.
I do care about my whitespace and it does have meaning. Throwing it all away on safe sounds annoying.
Sure, but it sounds like you have a problem with prettier/white space auto-formatters in general? My comment was assuming you want an auto-formatter, and I gave the reasons for why you’d want it to apply on save vs format-on-commit or w/e.
I care about my vertical white space (eg: grouping some things together gives me some logical separation).

I don't care about horizontal white space. That should just properly get indented and spaced into a consistent block format.

Fine, I do. Also most tools don't get the difference between indenting and aligning and mess that up.
100% agree. Autoformatting is such a great feedback mechanism.

Did this not indent where I expected? Did it not change the formatting?

Do I use it as a crutch? Maybe. Would I be a better programmer if I didn't use it? I doubt it.

Fight the power, setup format on checkout with your own rules.