[whstl]

+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.

[harimau777]

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.

[Hasnep]

Which tools are you using that allow custom linter rules? I'd like to try this on some of my repos

[whstl]

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.

[DanielHB]

eslint and biome allow custom lint rules and it is not that hard to write new ones yourself.

[schwartzworld]

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.

[fenomas]

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.

[waisbrot]

+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.

[harimau777]

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.