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

CI: add skywalking-eyes license header check #2215

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

homksei
Copy link
Contributor

@homksei homksei commented Dec 8, 2024

Description

Added skywalking-eyes license header check:

  • .github/workflows/skywalking-eyes.yml workflow (actually, the check itself)
  • .github/.licenserc.yaml config for checker

Additionally:

Copyrights in some files were updated:

  • Updated header for: .ci/scripts/install_windows.bat

  • Added license header to the following files:

    • .github/workflows/renovate-validation.yml
    • .github/ISSUE_TEMPLATE/-bug_report.md
    • .github/Pull_Request_template.md
    • .github/issue_template/Bug_report.md
    • CODE_OF_CONDUCT.md
    • README.md
    • daal4py/README.md
    • examples/notebooks/README.md
  • Fixed framing .rst files in docs:

    • doc/daal4py/algorithms.rst
    • doc/daal4py/contents.rst
    • doc/daal4py/data.rst
      ...
    • doc/sources/404.rst
    • doc/sources/algorithms.rst
    • doc/sources/array_api.rst
      ...

PR should start as a draft, then move to ready for review state after CI is passed and all applicable checkboxes are closed.
This approach ensures that reviewers don't spend extra time asking for regular requirements.

You can remove a checkbox as not applicable only if it doesn't relate to this PR in any way.
For example, PR with docs update doesn't require checkboxes for performance while PR with any change in actual code should have checkboxes and justify how this code change is expected to affect performance (or justification should be self-evident).

Checklist to comply with before moving PR from draft:

PR completeness and readability

  • I have reviewed my changes thoroughly before submitting this pull request.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation to reflect the changes or created a separate PR with update and provided its number in the description, if necessary.
  • Git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details).
  • I have added a respective label(s) to PR if I have a permission for that.
  • I have resolved any merge conflicts that might occur with the base branch.

Copy link
Contributor

@icfaust icfaust left a comment

Choose a reason for hiding this comment

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

My review focusses on the github action script, since everything else is conformance. One small question about the linter.

.github/workflows/skywalking-eyes.yml Outdated Show resolved Hide resolved
.github/workflows/skywalking-eyes.yml Outdated Show resolved Hide resolved
.ci/scripts/install_windows.bat Outdated Show resolved Hide resolved
doc/daal4py/contents.rst Show resolved Hide resolved
Comment on lines 17 to 24
on:
pull_request:
branches:
- main
push:
branches:
- main
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to add same check for release branches too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it is not necessary, as release branches are usually created from a commit in the main branch, which should have already been verified by that time.

However, I have added it just in case.

@homksei homksei force-pushed the chore-add-license-check branch from 511efe4 to ec6e062 Compare December 9, 2024 12:46
@homksei homksei requested a review from Alexsandruss December 9, 2024 12:51
Copy link
Contributor

@icfaust icfaust left a comment

Choose a reason for hiding this comment

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

I think its good to go, for extra credit it may be good to test a failure to test the writing function I asked about. (since this and uxlfoundation/oneDAL#2867 don't show one in their PR histories).

@homksei homksei force-pushed the chore-add-license-check branch 3 times, most recently from 0f8c687 to 968e3ec Compare December 10, 2024 14:24
@homksei homksei force-pushed the chore-add-license-check branch from 968e3ec to dfbf1a9 Compare December 10, 2024 14:28
@homksei homksei merged commit 0b74e74 into uxlfoundation:main Dec 10, 2024
13 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request infra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants