Skip to content

Use "clean" LICENSE and NOTICE in published jar artifacts #1292

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

Merged
merged 1 commit into from
Apr 15, 2025

Conversation

jbonofre
Copy link
Member

@jbonofre jbonofre commented Apr 2, 2025

In preparation of the next releases (including jar artifacts publication), this PR uses "clean" LICENSE and NOTICE in published jar artifacts, and "regular" location.

@jbonofre jbonofre marked this pull request as ready for review April 7, 2025 03:14
@jbonofre jbonofre requested a review from snazy April 7, 2025 03:14
Comment on lines +203 to +204
--------------------------------------------------------------------------------

Copy link
Member

Choose a reason for hiding this comment

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

I don't mind adding LICENSE file(s) to the non-root projects, but CopiedCodeCheckerPlugin should then also be adopted to check those files as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, good point.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just a note: the reason I'm adding LICENSE to the non-root projects is because they are distributed (on Maven). If not distributed, it won't be necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

I registered integration-tests in the check. Please let me know if you want me to use a more global pattern to register all "non-root" project, or if only integration-tests is good enough for now.

Copy link
Member

Choose a reason for hiding this comment

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

Not fully what I meant.
The CopiedCodeCheckerPlugin needs to look into the special LICENSE file and verify that it contains the necessary mentions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah ok, you mean having "special" configuration of the checker plugin in the project. Catcha. Let me do that.

Copy link
Member Author

Choose a reason for hiding this comment

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

@snazy do you mind to take a new look to be sure I understood what you meant correctly ? Thanks !

Copy link
Member

Choose a reason for hiding this comment

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

No, I mean updating the functionality in the CopiedCodeCheckerPlugin

Copy link
Member Author

Choose a reason for hiding this comment

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

I pass in the configuration of the polaris-test (location of the LICENSE file). I guess you mean to check if project has LICENSE file and fallback to the default one right ?

@jbonofre jbonofre requested a review from snazy April 9, 2025 15:40
@jbonofre jbonofre force-pushed the jar-license branch 3 times, most recently from 8005359 to ef6b9fe Compare April 9, 2025 16:43
dimas-b
dimas-b previously approved these changes Apr 9, 2025
Copy link
Contributor

@dimas-b dimas-b left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@github-project-automation github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Apr 9, 2025
flyrain
flyrain previously approved these changes Apr 9, 2025
@jbonofre jbonofre dismissed stale reviews from flyrain and dimas-b via 897afc7 April 10, 2025 16:12
@jbonofre
Copy link
Member Author

FYI, this PR is blocking the release, can you please provide an update quickly ?
Regarding @snazy comment, I think I include the needed configuration to check in the polaris-test project, if it's not good enough, I propose to merge this PR and address @snazy comment in another PR.
Toughts ?

@jbonofre jbonofre merged commit 0fe89ec into apache:main Apr 15, 2025
5 checks passed
@github-project-automation github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Apr 15, 2025
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.

4 participants