DEV Community

Cover image for It's okay to commit lint fixes
Jan Küster
Jan Küster

Posted on

It's okay to commit lint fixes

Trigger-warning, this is all highly opinionated. If you feel different about it, let's discuss 😄

It bases on my assumption, that a "clean" git history is something I might want to achieve but I don't necessarily need to achieve and the tradeoff between what I get by a clean history and the work I have to do for it seems to be negative for teams below enterprise-level size. Therefore commits about "lint fixes" are totally okay imo.

Here are some rather technical arguments:

"fix(ui): eslint compliance"

  • if you use a convention then you can easily filter these commits:

git log --invert-grep --grep="eslint compliance"

  • you can still use git squash-merge if a clean history is of most importance

Here are some rather human arguments:

  • for newcomers and beginners it's often much easier to first commit the work they have done (feel safe; make sure things a working) and then apply formatting, refactoring etc.
  • convention over configuration does imo also apply to the dev environment, which is why I regularly omit pre-commit hooks
  • I also like to avoid to put my juniors into a skinner-box that only lets them pass, once they behave as I intend them to; rather I'd like them to learn conventions

What do you think? Something is missing? Is this unprofessional and I should never step a foot into your company's door? Let's discuss 😄

Top comments (10)

Collapse
 
nlxdodge profile image
NLxDoDge

I agree with this, but please for the love of the PR gods. Make a separate PR for linting if it lints the whole project 😢 (for example line ending changes between MAC/Unix/Windows) Happends all the time in our company.

Collapse
 
jankapunkt profile image
Jan Küster

That's actually quite a good convention, reviewers will thank you for it!

Collapse
 
aneshodza profile image
anes

IMO think style checks should be a pre-commit/pre-push hook, so stuff like that is never even necessary. And on top, when merging PRs I also prefer to squash first. That just leaves a clean git history on develop.
That's just my opinion tho

Collapse
 
jankapunkt profile image
Jan Küster

Of course, your opinion this is a valid approach! However, I'd like to know why a clean git history is of such importance to you / your team?

Collapse
 
tandrieu profile image
Thibaut Andrieu

To identify the culprits and punish them appropriately !

Joking aside, unfortunatly it may be one of the reasons. We had "A guy" that were known to write let say "useless test case that regularly randomly fail". The Guy leaved the company, but the tests remained. And regularly, when we fell on one of its test (because its name was written in history), we didn't lose time fixing it. We just removed it, get back to initial requirement (because attached ticket is written in history) and rewrite it from scratch.

Having linear history also greatly help to find regression, using git bisect like @aneshodza said or by just reading modification in history. If you had squashed the whole branch when merging, you end up with a huge useless commit. If your history is interleaved with lots of "fixup typo" or "format code" commit, it requires more intellectual effort finding the modification that may have break something. If your history is filled by "fix previous modification" commits, then git bisect is not possible at all.

In the end, it depends on your way of working. If you never go back in time, then having a clean history is maybe useless. But if you work on a big legacy codebase, or with a lot of turnover, then having the step that developer had in mind when he did its job may greatly help debugging.

Collapse
 
aneshodza profile image
anes

Having the hooks isn't about the history but rather about maintaining a clean codebase. Having said that me and people I work with tend to put a rather big importance on clean git histories for various reasons. The ones I was able to really feel where:

  • Code is easier to maintain/expand. If I need to expand on a feature I can simply check the git history and save myself a load of work.
  • Being able to identify bugs easier. Having a clean history let's you work more efficiently against bugs. E.g. is the command git bisect, which lets you binary search trough the commit history to find bugs. I won't go into detail about the command, but a clean git history helps a lot in stuff like that
  • Easier onboarding. When a new person is added into the codebase, they can build a better understanding for the codebase by looking trough the git history.
  • And a lot more

TLDR: A clean git makes maintainable and understandable codebases

Collapse
 
mistval profile image
Randall

In any team project, commits that only fix linting shouldn't be necessary, because your CI/CD setup should enforce the style rules and not allow un-linted code to be merged to the main branch.

In my personal projects, sometimes I don't bother to set that up, and sometimes I do forget to lint, and push a commit to fix that. But for any team project, I think pre-merge checks in CI/CD are essential.

Collapse
 
jankapunkt profile image
Jan Küster

If you have a merge request open then you also need to push code to it, so it's okay to push commits with lint fixes into these branches (that will get delete anyway after merge/squash), right?

Collapse
 
mistval profile image
Randall

Right, I should have also mentioned that I always squash MRs.

Collapse
 
benhocking profile image
Ben Hocking • Edited

I would agree, except that rather than it being "okay", it's preferred to commit your lint fixes separate from enhancements or bug fixes, as it makes it much clearer what the purpose of the changes are.