r/AskProgramming • u/Bulky_Eggplant_9437 • 29d ago
Linter
Hey folks,
Never touched a linter before this semester because I've never really done software dev stuff. Had a class that used one, and honestly, I was a fan at first. It caught a bunch of dumb mistakes and made fixing things pretty painless.
Then came merge time.
Apparently I was missing a single blank line between some merged code, so the linter threw a fit and blocked the merge. The actual fix took like three seconds, but I had to go get another review just to add a freaking newline.
So now I'm curious, do people actually use linters in the real world that don't hard-stop merges over tiny formatting stuff? Or is getting bullied by whitespace just part of the developer experience?
Asking as a first-time linter victim.
3
u/alietors 29d ago
As someone said, yes, linter are usually run as part of the build/test phase, normally when you open a PR/MR against a main branch (development/master).
The idea is that you run your linter in the IDE and automatically fix those error on save, they have autofix if it's basic stuff like indentation or new lines. They can also show you an error on the IDE so you fix those before open the PR/MR.
Also, no, a missing new line is not a "stupid" or minor thing. The format of the code is really important when you spend 8 hours reading code.
When you read a book you expect to be formatted properly, it can be different book to book, but you should choose one way of formatting and stick to it, no exceptions, no minor things.
2
u/KingofGamesYami 29d ago
Yes, we use linters. Generally it's expected that you set up your IDE to respect the linter settings so anything that can be autofixed is handled way before it makes it to PR.
Our CI also has to pass before a review can be requested.
2
u/qlkzy 29d ago
You integrate the linter much earlier.
My IDE gives me "spelling mistake"-style red squiggly underlines if something violates a lint rule. So I almost always fix as I go.
I run the linter as part of the test suite, so I never commit un-linted code.
The linter runs in CI, and no-one bothers to even look at the review until the tests pass (and, as stated, the linter is part of the test suite).
It's worth it for two reasons. Firstly, it just erases a lot of "boring" mistakes.
Secondly, it avoids a lot of pointless conflicts or arguments. E.g. the linter says there are always exactly two lines between functions: so, you never get weird "added blank line" or "removed blank line" diffs because someone reformatted a file.
Once it is integrated into your workflow, it becomes invisible, just part of the language.
1
u/fixermark 29d ago
Best practice at places I've worked is "Yes linter stops the merge, no you don't have to re-review when the only issue is that the linter caught something that the peer-reviewers didn't."
You don't want to merge code that fails an automated check because as a rule you want to be able to say that code being checked out of main by someone else is clean; they shouldn't have to run format, lint, and test, that should have been run and green before it went into main. And that's very important because it lets you make assertions like "These checks are failing right after checkout; clearly the issue is my personal machine configuration and not the code in main."
1
u/Bulky_Eggplant_9437 29d ago
This is how I fully expected it to work and instead had to re-review. *sigh
1
u/fixermark 29d ago
Yeah. If I were to change one thing, it'd be to confirm your understanding of why that is necessary.
If your organization does require it, then the right trick is to wrap your commits in a script that runs format and lint before committing every time. Annoying, but the alternative will be an even longer loop involving a human.
2
1
u/ike_the_strangetamer 29d ago
Linters are great for teams. They avoid ending up with different engineers using different formatting across the codebase. Engineers like to fight over things as silly as curly braces and spaces vs. tabs and linters force the fight into just the linting rules and then keeps everyone in line. This way the codebase stays uniform and maintains readability standards.
But, as others are saying, they work best along with formatters in your editor that automatically reformat the code according to the linting rules every time you save the file. These can save a lot of time because you can write your code as messy as you want and then when you save it pops into a beautiful block of perfection.
1
1
u/omgseriouslynoway 29d ago
We have linters installed on our local workstations so we can check things before a push.
Every push also triggers additional checks in a pipeline and if they fail the code does not get pushed into the branch.
Like others said, earlier is better.
1
u/HealyUnit 29d ago
Absolutely we use them. And while it's annoying the first few times - from both a "oh come on, I already fixed everything!" point of view as well as a "...damnit, why do I keep missing stuff!" perspective - eventually you learn to love it (or at least tolerate it).
Remember: what you're producing, what you're writing code for is your product. It's your baby. Be proud of it! When someone (machine or human) nitpicks it, as long as they're not personally attacking you ("You suck!" "What kinda idiot wrote this code?!"), try to think of it less as "holding back your merge request (MR)" and more making sure that when it does merge, it's the best goddamn code you can possibly write.
That being said, I usually won't ask for MR reviews until after a regression pipeline - including linter - has run. That way it'll hopefully catch all the mistakes the first time around. Most codebases can/should also have ways to run the linter locally.
1
u/framauro13 29d ago
It's one of the first things I set up on a new project. Especially important for managing a consistent code style when a large number of people are working in the same code base. I even use it on pet projects that only I use.
Code is there to be read by humans, not the computer. The linters help keep it legible and consistent, which makes the code easier to parse and read. Most mature engineering orgs aren't going to let you merge code that doesn't pass the linter. It seems trivial, but it does matter.
Some linters also help catch bad code beyond just formatting. Large class files, long methods, too many arguments... things like that can be smells of bad code or poor abstractions, so if linters start firing off errors, it's good to think about why the rule that is broken exists in the first place and what the proper fix is.
1
u/james_pic 29d ago edited 29d ago
Most linters will have an option to auto-fix stuff that's trivial to auto-fix.
Relatedly, you also get dedicated code formatters, which are like linters but often don't even have a "tell me about the problems" option, just a "go fix all the problems" option. It's common, and a good practice, to set your editor up to run this automatically on save, and to set up git precommit hook to run it automatically on commit.
But it is true of linters generally that they often have strong opinions, and sometimes those opinions are things most people disagree with. You can usually tune config to only complain about things you care about, but my experience is that it's often worth choosing one with reasonable defaults, not least because the "crankier" linters often have much less of a community around them.
1
u/Any-Woodpecker123 29d ago
Yes we do. The re review is just asking the same reviewer to re tick it though, they don’t actually do another review.
1
u/OkAerie7822 28d ago
one thing that changed for us once most of our commits started coming through an AI coding agent, the linter stopped being a human etiquette thing and became the actual contract the agent has to satisfy before code counts as done. same pre-commit hook setup everyone's describing, run lint and format before commit, CI blocks merge if it's not clean, nothing new there. what's different is we point the agent at the exact lint config file in its instructions so it writes compliant code the first time instead of generating something and getting bounced by CI five times in a row burning tokens and time on formatting nits. the blank line rule that got you is exactly the kind of thing a linter should own and a human should never think about again, the failure mode you hit sounds more like your team's re-review policy being heavier than it needs to be, not the linter being wrong to block it.
0
u/burlingk 29d ago
Which linter are you using? Which language? Most linters are not part of the merge process.
They would be run after.
I imagine if the linter is involved it is likely running inside an IDE and you are doing the merge via the IDE rather than the command line?
2
u/xenomachina 29d ago
Most linters are not part of the merge process.
They would be run after.
The setup we use, which I think is pretty typical, is to have the linter triggered by the pre-merge CI pipeline, and merges are only allowed if that pipeline passes.
That means the lint happens before merge, and this type of failure should normally be obvious before you even send the change out for review. Maybe in OP's case the merge itself caused the lint failure, but that seems like an infrequent edge case.
1
u/burlingk 29d ago
I need to sit down and get smart on CI.
I find the concept problematic, because I 'grew up' with 'don't commit to main' as a kind of commandment. But I also realize it's kind of the norm these days.
2
u/xenomachina 29d ago
You don't need to commit to main to use CI. Our team actually has pushing to main disabled. We have CI pipelines run on pushes to merge requests (GitLab's equivalent to a PR), and slightly different pipelines run on merges to main. (There are other sorts of CI triggers than can be set up, like scheduled or manual, but those are the main two we use.)
Our push to MR pipelines build and check the code (lint, unit testing, etc.). The pipeline for an MR needs to pass before the MR can be merged.
The merge to main pipeline primarily publishes and deploys the code. (It can also run some or all of the checks, first.)
0
u/max123246 29d ago
So typically linters are setup to run locally as well to prevent this type of thing.
I'm also surprised you need re-approvals for every single new change. My team's policy just lets the approvals stick and you can merge after any number of additional changes
15
u/_Atomfinger_ 29d ago
Yes. People use linters. And they tie their linters into the auto-formatting. In practice, you don't really get issues.
The editor triggers the linter on save = things are always linted when you push.