DEV Community

Ben Halpern
Ben Halpern

Posted on

Can forced linting surpress contributions when linting is first introduced?

We recently introduced forced linting to the views and enforce it via a pre-commit hook.

Before this change, we’d been completely lacking in view file linting, so when someone makes a small change to a view file now, they will be met with a laundry list of small changes.

Most changes are things like asking the developer to make an afjustment such as changing <%=user.username%> to <%= user.username %> (spaces preferred to no spaces) as per the style guide.

I am concerned that this could add too much barrier for small contributions, even though we would love to get our views consistently linted eventually.

Here was the issue in the project, but I’d love any general input on the topic here.

Could view linting be suppressing contributions? #1828

I have the feeling that view linting could be presenting a pretty strong burden to contributions. You could make a small contribution and get hung up with a huge linting fix chore as it currently stands.

I just feel like as an outsider without context it could add a bit too much burden at the time being. Thoughts on taking it away as a blocker to pushing and leave it as something folks can voluntarily make progress on until the app is at a closer place to being fully linted on the view with our preferred styles?

I’d love to hear about how you’ve dealt with this in the past.

Top comments (15)

cher profile image

If the linting can be auto-fixed on pre-commit, it's valuable. It removes negative nit comments that may deter people from contributing. If they can't be fixed with a linter, I'd question the validity of the style preference from outside--and instead defer any linting failures to be fixed by regular contributors.

suprnova32 profile image
Patricio Cano

This is why I love the Elixir formatter. When you run it, it automatically fixes any offenses. You can add it as a pre-commit hook and your work will always be formatted. You can also run a check on CI to make sure all code is formatted.

I like that it is really opinionated and not really configurable. It gives you a style guide for the entire Elixir ecosystem.

I’d wish there was something like this for Ruby. Rubocop can be such a pain...

jf1 profile image
Jed Fox

Check out Prettier’s Ruby plugin:

Thread Thread
suprnova32 profile image
Patricio Cano

I've heard about Prettier before, but it is still not quite the same as the Elixir formatter. For Elixir, it is a part of the language, and many libraries use it. This means that there is a cohesiveness in most Elixir codebases.

Ruby, while it does have some efforts, does not have a formatter as a core part of the language.

paulleclercq profile image
Paul Leclercq

Hi Ben,

I don't think it will surpress contributions.
But contribution may take longer, as I code in my spare time, it's hard to keep the motivation on the long run, as it adds a bit more work.
I've been working on this PR for ages now, and the last task is to solve this rule

Wish me luck :D

thomasjunkos profile image
Thomas Junkツ

I see this is a non issue: Todays tools should autoformat/-lint code.

This issue should not even occur. And where the linter complains it is necessary so.

It is not so much a burden - the reason for linting is not bugging people for no reason - it comes down to: if you want to contribute, you have to play by the rules and code along our standards.

vcarl profile image
Carl Vitullo

I've always felt that code style changes should be opt-in at a file level, or even sub-file for particularly large files. Linting is valuable, but fighting the linter when you're trying to solve an unrelated bug makes it harder to compare the diff during code review.

Having it be opt-in lets you change over 1 file or major section at a time, without burdening small PRs. It even opens the door to a lot of minor contributions, as the process of fixing lint issues in a large number of files is a pretty large chore that doesn't take any particular knowledge or context of the codebase.

sudiukil profile image
Quentin Sonrel

IMO it may (not sure) suppress a few contributions but I don't think it'll have a meaningful impact at all. I personally don't contribute to (yet, lacking time and energy) but I wouldn't be bothered by this if I was. I think it's very good to enforce some rules for your code (had a discussion about that with a co-worker this very morning) and linters are a very good way to do so.

itsasine profile image
ItsASine (Kayla)

If using precommit hooks, I would assume a --fix (or whatever the ruby equivalent is; I live in ESLint land) would be appended to the script so it runs but only stops things if there's human intervention needed. I don't think it would stop people from contributing, though, since if you put effort into contributing code, I'd think you would want to do it right so it gets merged in. It just might make things take a bit longer.

itr13 profile image
Mikael Klages

I like the way go does it, where all formatting happens automatically with go format (or on save with some IDEs). For linting in terms of error-checking specifically, I guess I'm more in favor of the program not being run if a big issue is detected, similar to what happens during compiling in compiled languages.

moopet profile image
Ben Sinclair

If there's nothing broken about it (i.e. it's only linting the committed code and not the whole shebang) and if it's possible to lint your codebase (I'm looking at frameworks like Drupal...) then I see nowt wrong with it. I've been spare-timing a project to make a generic linter that figures out what needs to be checked in what language and pulls appropriate dockerised linters to do it, with a view to forcing it in git hooks.

Git hooks can always be bypassed in an office where devs can chat to one another. It's slower over the net though.

alephnaught2tog profile image
Max Cerrina

If it's something likes adding spaces, etc., is there a way to just set up a hook on CI so it does it for folks?

codingmindfully profile image
Daragh Byrne

I love forced linting. If you want to develop the disciple required to produce consistently clean code, you’ll love it too!

meain profile image
Abin Simon

It's better for it to fail in the CI than have it fail in a git hook. That way the commit still comes through but you can ask them to fix the linting issues.

joelnet profile image
JavaScript Joel

Lint first, ask questions later.