Add git hook(s) to automate linting before or during a push

[fbuys]

What

More of a DX feature.
Perhaps we can add a git hook to automate linting before or during a push.

We could use something like overcommit to ensure that rubocop passes before a commit is made for example.

Why

This would prevent changes being pushed just to realize after CI ran that there is a lint issue.

I am not sure if adding a developer dependency is a problem, but would like to hear the communities take on this idea.
I can't say for sure how often this is a problem, and since we squash commits it is not a huge issue.
I do feel like it would improve the developer experience and it will prevent failed CI runs due to simple liniting issues.

Alternatives

We don't necessarily need overcommit, I am sure it is possible to add a hook manually by hand.

[thdaraujo]

I don't have an opinion on this, so would love to hear what others think about this.

On one hand, it can be a bit annoying to have to lint your code just to be able to commit a completely unrelated change (like when you're doing a partial commit).

For me personally it's not a big problem, but I tend to run tests all the time. The default rake task also runs rubocop, so running the test suite always catches this type of error.

On the other hand, I know how annoying it is when the CI fails just because you forgot to run rubocop.

I guess overcommit could be added as a dev-only and optional dependency.

Or, even better, we could add an example hook and tell people to use it if they want to. Then we could explain how it works on CONTRIBUTING.md. What do you think?

[ChaoticBoredom]

Who's making commits w/o running their tests? XD
That said, I'm all for helping folks recognize in advance if their commit is not going to work before they make it. I also know folks that are against anything that adds overhead to their workflow. I think adding it to CONTRIBUTING.md would be an excellent compromise, allowing folks that get hit by failed lints to help themselves.

[fbuys]

Or, even better, we could add an example hook and tell people to use it if they want to. Then we could explain how it works on CONTRIBUTING.md. What do you think?

I like this idea too

[thdaraujo]

Or, even better, we could add an example hook and tell people to use it if they want to. Then we could explain how it works on CONTRIBUTING.md. What do you think?

I like this idea too

Cool!

Okay, so it sounds like what we want here is to:

1. add an example of a pre-push hook
2. the example hook runs the linter before a push, and if errors are found, the push is aborted
3. add a section on CONTRIBUTING.md explaining what to do if they get a linter error on CI, how to enable and use the hook if they decide to do it, and how to disable the hook or skip it.

[akanshak984]

@thdaraujo I would like to work on this.

[stefannibrasil]

I do feel like it would improve the developer experience and it will prevent failed CI runs due to simple liniting issues.

I disagree that simple linting issues are a small thing in the developer world. I prefer commits that are both testes and linted.

Like @thdaraujo mentioned, our specs run the tests and rubocop, so the errors are available locally before pushing. The feedback loop is there to prevent pushing broken commits (although there is nothing wrong with that either).

I don't like the idea of only running the specs if the linter passes because I see both necessary to follow the good practices. However, if this is an optional feature that is disabled by default, adding it to the docs is fine with me.

[fbuys]

the example hook runs the linter before a push, and if errors are found, the push is aborted

Could even just run rake so that the specs and rubocop run before pushing.

[thdaraujo]

@thdaraujo I would like to work on this.

thanks! 🙏

[mateusdeap]

Hey @akanshak984! Were you able to work on this? Because I can also tackle it and open the PR if needed.

[akanshak984]

Hey @akanshak984! Were you able to work on this? Because I can also tackle it and open the PR if needed.

Hi @mateusdeap Not much! But I'm working on it.