Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(jenkins): support check license with go-licenser #518

Closed
wants to merge 1 commit into from

Conversation

v1v
Copy link
Member

@v1v v1v commented Jul 4, 2019

Highlights

  • Enable the go-licenser stage in the pipeline
  • Enable the go-licenser goal in the makefile.

Tasks

@beniwohli
Copy link
Contributor

@v1v can you give a short explanation on the benefits of using go-licenser instead of the relatively simple bash script?

One benefit of the bash script is that we can use it very easily in a pre-commit hook:

- repo: local
hooks:
- id: license-header-check
name: License header check
description: Checks the existance of license headers in all Python files
entry: ./tests/scripts/license_headers_check.sh
exclude: "(elasticapm\/utils\/wrapt\/.*|tests\/utils\/stacks\/linenos.py)"
language: script
types: [python]

If we switch to go-licenser, it would be great if that could also be applied to the pre-commit hook somehow to ensure parity between the two checks.

@v1v
Copy link
Member Author

v1v commented Jul 9, 2019

That's a fair question.

The major benefit is to provide the same toolchain for all the agents if possible, I don't say the go-licenser is the one to go, but as it's already a tool provided and used, we could potentially extend it for the rest of agents and being able to have the same common toolchains.

At the moment the current implementation doesn't provide all the requirements for the python agent though. That's one of the reasons I raised this PR as a draft. Do you think I should move this conversation to an issue firstly rather than in this PR?

Are there any concerns in your end?

Thanks

@beniwohli
Copy link
Contributor

I agree that harmonizing the license check would be great. But if it hugely complicates go-licenser just to be able to handle the Python agent (AFAIK, most other Elastic projects share the same Apache2 license), it might not be worth it. And as I said, having the check work in a pre-commit hook is pretty much a requirement for me.

@v1v
Copy link
Member Author

v1v commented Nov 21, 2019

I'll close this PR for the time being, and once we go the go-licenser in place we can revisit it.

@v1v v1v closed this Nov 21, 2019
@zube zube bot reopened this Nov 21, 2019
@zube zube bot closed this Nov 21, 2019
@zube zube bot reopened this Nov 21, 2019
@zube zube bot closed this Nov 21, 2019
@zube zube bot removed the [zube]: In Review label Nov 21, 2019
@zube zube bot added the [zube]: Done label Nov 21, 2019
@zube zube bot reopened this Nov 21, 2019
@zube zube bot closed this Nov 21, 2019
@zube zube bot reopened this Nov 21, 2019
@zube zube bot closed this Nov 21, 2019
@zube zube bot reopened this Nov 21, 2019
@beniwohli
Copy link
Contributor

zube didn't agree with having this closed. Let's try again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants