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: Add license utility to chectl to validate and add license header to codebase #1278

Closed
wants to merge 1 commit into from

Conversation

flacatus
Copy link
Collaborator

@flacatus flacatus commented Jun 4, 2021

What does this PR do?

This PR add a new automation to chectl to check and add license headers to codebase. Also this PR fix all incorrect license headers.

Refference issue: eclipse-che/che#19647

Screenshot/screencast of this PR

What issues does this PR fix or reference?

How to test this PR?

Download current branch and execute:
For check release:

/bin/bash .ci/check-license.sh --check-license

To add new license header to a file which doesn't contain a license:

/bin/bash .ci/check-license.sh --add-license

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

@openshift-ci
Copy link

openshift-ci bot commented Jun 4, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: flacatus

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Collaborator

@benoitf benoitf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not using eslint https://www.npmjs.com/package/eslint-plugin-header ?

IMHO typescript projects should use eslint plug-ins so it's handled in IDE as well

@openshift-ci
Copy link

openshift-ci bot commented Jun 4, 2021

@flacatus: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/v7-chectl-e2e-olm-installer 37ffe7b link /test v7-chectl-e2e-olm-installer
ci/prow/v8-chectl-e2e-olm-installer 37ffe7b link /test v8-chectl-e2e-olm-installer
ci/prow/v7-chectl-e2e-operator-installer 37ffe7b link /test v7-chectl-e2e-operator-installer
ci/prow/v8-chectl-e2e-operator-installer 37ffe7b link /test v8-chectl-e2e-operator-installer

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@mmorhun
Copy link
Contributor

mmorhun commented Jun 4, 2021

@flacatus @tolusha @benoitf I have another question to discuss. This PR forces all headers to have date 2019-2021. However, some files haven't been changed since, say, 2020 and some were created in 2021. To my mind, it is not right to force dates in license headers, as they should describe real state of the files. WDYT?

@tolusha
Copy link
Collaborator

tolusha commented Jun 4, 2021

It isn't a problem to update year.
We used to do it in old che repository every year.

@@ -1,6 +1,5 @@
#!/bin/bash
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe for shell script we have to save #!/bin/bash ? I guess they will be broken without this directive...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My mistake

@@ -1 +1,14 @@
/**
*
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra empty line

@flacatus
Copy link
Collaborator Author

flacatus commented Jun 7, 2021

Closing in favor of #1280

@flacatus flacatus closed this Jun 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants