antrea-io / antrea

Kubernetes networking based on Open vSwitch
https://antrea.io
Apache License 2.0
1.66k stars 366 forks source link

Utilize git commit hook to reduce the usage of github runner #3611

Closed luolanzone closed 2 years ago

luolanzone commented 2 years ago

Describe the problem/challenge you have When we have multiple PRs running, the time of the workflow job spending on waiting available github runner might be quite long. but sometimes, there are just some static or doc check failure which are actually can be run locally before push. we can actually use pre-push hook to do some checks before push which can help on reducing unnecessary force push and reduce the usage of github runner.

Describe the solution you'd like git has a way to fire off custom scripts when certain important actions occur, they are client-side hooks and server side hooks, we may utilize client-side pre-push hooks to do some statics/docs check etc before any push, below are the candidates for pre-push check.

  1. make golangci
  2. make verify

it would be better if developer can set the hook dynamically since the change might not be related with doc etc.

Anything else you would like to add?

Reference: https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks https://githooks.com/ https://www.git-scm.com/docs/githooks#_pre_push

mohitsaxenaknoldus commented 2 years ago

Hey, can I work on this?

luolanzone commented 2 years ago

@mohitsaxenaknoldus sure, thanks. @antoninbas @tnqn do you have any comment or suggestion for this proposal?

antoninbas commented 2 years ago

I don't have a strong opinion on this, it's ok to provide hooks for developer to use. However, I think we need to keep the following in mind:

luolanzone commented 2 years ago

thanks for the suggestion, @antoninbas , I think maybe we can put those pre-defined hooks into a template folder and add a new make target to allow user to copy hooks from antrea repo to local .git/hooks/, and provides a few steps in CONTRIBUTING.md to remind user about these hooks.

antoninbas commented 2 years ago

this sounds fine to me

mohitsaxenaknoldus commented 2 years ago

PR: https://github.com/antrea-io/antrea/pull/3669 @luolanzone

antoninbas commented 2 years ago

@mohitsaxenaknoldus @luolanzone I have an idea for a follow-up PR: add a hook to verify that commits are signed when pushing. That's a very common "mistake".

luolanzone commented 2 years ago

@antoninbas, I agree with you, it's indeed a common 'mistake', it's a good idea to add a commit to check sign-off, maybe we can also add sign-off info automatically if there is no one in the commit?

luolanzone commented 2 years ago

Hi @mohitsaxenaknoldus I saw you created a new issue #3704 for sign-off check commit, are you going to work on this? Could you please refine the description of the issue to make sure you provide clear idea? or maybe link it to this issue with a simple background intro.

antoninbas commented 2 years ago

I wouldn't recommend signing off commits automatically. The sign off means that the contributor acknowledges the DCO, and I don't think this should be automated.