loading...

Can forced linting surpress contributions when linting is first introduced?

ben profile image Ben Halpern ・1 min read

We recently introduced forced linting to the dev.to 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.

Posted on by:

ben profile

Ben Halpern

@ben

A Canadian software developer who thinks he’s funny. He/Him.

Discussion

markdown guide
 

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.

 

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

 

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.

 

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 eslint.org/docs/rules/no-await-in-...

Wish me luck :D

 

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.

 

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.

 

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.

 

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

 

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.

 

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.

 

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

 

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?

 

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.