ekino / jcv

JSON Content Validator (JCV) allows you to compare JSON contents with embedded validation.
https://ekino.github.io/jcv
MIT License
21 stars 0 forks source link

feat(check commit): add pre-commit and hook #12

Closed saugereau closed 4 years ago

saugereau commented 4 years ago

Use npm to have a feature of git hook checking for conventional commits. Pro : a ready-to-use solution Cons : introduce package.json in the project But with this kind of solution we can have the same requirement on commits for front and back projects ... Maybe will it be interesting to add this kind of feature in our gradle-quality-plugin ...?

leomillon commented 4 years ago

I'm not really a fan of having to install NPM to just have commit format validation...

Especially handled by Gradle on the build task:

So this feature could be interesting if it was included inside the git repository directly (without NPM tooling) or maybe in the CI process (validating the commit history during the pull-requests).

@clemstoquart , @celcius112 , any other feedback regarding this "feature"?

clemstoquart commented 4 years ago

Hi guys,

I agree with @leomillon , I'm not comfortable to delegate that kind of check to a node module.

saugereau commented 4 years ago

Wouldn't it be interesting to develop our own plugin ? :)

leomillon commented 4 years ago

I think it should probably be a git hook configuration only (nothing managed by Gradle or any other building tool)

And finally, even if it was a git configuration only, the pre-commit hook would quickly annoy me as I'm creating a lot of commit during my local development that does not always follow the "conventions" before I rework them completely for the final pull-request push so...

In my opinion, the only proper solution could be a custom scan of pull-request commits during the CI build...

celcius112 commented 4 years ago

I agree too with leo. It seems there already are solutions for enforcing git rules during the CI build. A shame that we don't have similar capabilities as in Gitlab for this. However as seb suggested it might be interesting to find a solution with Gradle, or event create our own solution. The advantage being that a plugin might be shared more easily with other projects

Crow-EH commented 4 years ago

Don't forget that using a standard commit linter is cool because we have access to standard rules, standard tools using standard commit formats etc. without reinventing anything.

I think "Where we plug it" (local exec with git hook VS PR CI) might be the most important question.

By the way @leomillon, you can use --no-verify to skip the pre-commit hook if necessary, hence the common practice of mirroring pre-commit checks with CI checks.

leomillon commented 4 years ago

@Crow-EH I understand the goal of this approach to have a "cleaner" commit historic but, on the day to day work process:

The overall picture, to me, seems a bit too "strict" with too many constraints and installation costs when we can just say: the merge-request is there to ensure that everything is in order to be merged in the project history.