search

k8ssandra / k8ssandra-operator

The Kubernetes operator for K8ssandra
https://k8ssandra.io/
Apache License 2.0
176 stars 79 forks source link

Pre-push hooks for codegen, linting, etc. #915

Open Miles-Garnsey opened 1 year ago

Miles-Garnsey commented 1 year ago

What is missing?

We have ongoing issues where linting, codegen, CRD generation issues only show up post-push in CI. This is annoying and results in excessive CI runs and devs needing to check CI after every commit before making tiny changes.

We should consider the use of pre-push hooks in git to vet all of these issues prior to them hitting CI.

┆Issue is synchronized with this Jira Story by Unito ┆Issue Number: K8OP-235

burmanm commented 1 year ago

Doesn't make test do all of that in any case?

Miles-Garnsey commented 1 year ago

No, at the very least it doesn't lint: test: manifests generate fmt vet envtest ## Run tests.

Additionally, this ticket is more about whether we should have certain makefile steps run as a pre-push hook moreso than which steps they should be. Right now, there are no pre-push hooks in this repo afaik.

burmanm commented 1 year ago

Okay, that was indeed only in cass-operator: test: manifests generate fmt vet lint envtest ## Run tests.

Personally, since I'm going to run make test in any case before committing, the additional hooks would just make me angry (mainly because linting seems to take ages on my Mac..). So I would probably first commit and then run the tests, which feels wrong.

In any case, I think the test missing lint is wrong - even if not mentioned in this ticket.

Miles-Garnsey commented 1 year ago

Right, we should have consistency between here and our private projects as well, agreed.

A pre-push hook seemed like the lightest weight way to do it since we shouldn't be pushing all that often (and CI will fail if we miss some linting/testing items anyway). But if others think it'll add overhead/too much wait time then I guess I can examine other options.