apache / skywalking

APM, Application Performance Monitoring System
https://skywalking.apache.org/
Apache License 2.0
23.85k stars 6.52k forks source link

[INFRA] license-eye support pre-commit hook #8710

Closed kwanhur closed 2 years ago

kwanhur commented 2 years ago

Search before asking

Description

After finish coding, always need to do license check on new files before push. So hope it can be done when commit.

Add one hook in pre-commit could be a solution.

Use case

place license-eye hook in .pre-commit-config.yaml, like

- repo: https://github.com/apache/skywalking-eyes
  rev: v0.2.0
  hooks:
    - id: license-eye-header-check
    - id: license-eye-header-fix

precommit install-hooks then every new file will be checked and fixed before commit.

Related issues

No response

Are you willing to submit a PR?

Code of Conduct

kezhenxu94 commented 2 years ago

@kwanhur how will you implement this feature?

kwanhur commented 2 years ago

Add .pre-commit-hooks.yaml into repo skywalking-eyes, its content like:

-   id: license-eye-header-check
    name: license-eye-header-check
    description: "Checks the files' license header"
    entry: license-eye header check
    language: golang
-   id: license-eye-header-fix
    name: license-eye-header-fix
    description: "Fixes the files' license header"
    entry: license-eye header fix
    language: golang

Similar configs for license dependency.

kezhenxu94 commented 2 years ago

That's not what I expect, skywalking-eyes itself is a tool, we can add .pre-commit-hooks.yaml in this repo, but I expect this is something our users can also use in their repos, for example, adding a command to automatically generate these files in users repo, e.g. license-eye hook init

kwanhur commented 2 years ago

If on this way, local hooks maybe the choice, but it'll loss autoupdate feature.

kezhenxu94 commented 2 years ago

In my opinion there is no way to have the autoupdate feature (or non-local hooks), license-eye finally will be shipped as a executable binary, how could that .pre-commit-hooks.yaml take effect?

kwanhur commented 2 years ago

According to local .pre-commit-config.yaml, pre-commit installs the binary with specified repo and revision, and then local hooks work on repo's .pre-commit-hooks.yaml base on hook id.

So the id in .pre-commit-hooks.yaml is important, every hook takes effect through its entry. entry means how to work on those matched files.

kwanhur commented 2 years ago

I place license-eye hooks as the below, it works well.

- repo: local
  hooks:
    - id: license-header-check
      name: license header check
      entry: license-eye header check
      language: system
      types: [file]
    - id: license-header-fix
      name: license header fix
      entry: license-eye header fix
      language: system
      types: [file]
wu-sheng commented 2 years ago

Should we close this?

kwanhur commented 2 years ago

If there's no need integrating .pre-commit-config.yaml into skywalking-eyes repo, close it.

wu-sheng commented 2 years ago

I think we don't need to do that. pre-commmit hook is very personal preference, we don't want to set a bar for it.