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

fix(binding/java): Overwrite default NOTICE file with correct years #2918

Merged
merged 2 commits into from
Aug 23, 2023

Conversation

tisonkun
Copy link
Member

@tisonkun tisonkun commented Aug 23, 2023

This closes #2916.

Verified locally. And yes, opendal-java contains built without any compile dependency on Maven Central.

FYI @markt-asf this should address the issue you spot :D

Copy link
Member

@suyanhanx suyanhanx left a comment

Choose a reason for hiding this comment

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

Thanks

@Xuanwo Xuanwo merged commit 5c8d9bb into apache:main Aug 23, 2023
24 of 25 checks passed
@raboof
Copy link
Member

raboof commented Aug 23, 2023

This is indeed a reasonable way to make sure the NOTICE contains correct years. For example, commons-collections does something similar.

An alternative approach is to make sure inceptionYear and project.build.outputTimestamp are set to a 'realistic' values before starting the release process. For example maven-remote-resources-plugin does this (automatically via maven-release-plugin, even - but you might not be using that).

The latter approach still has the timezone problem mentioned earlier - though it is much less likely to be encountered for 'realistic' dates. apache/maven-remote-resources-plugin#39 should fix it.

@tisonkun tisonkun deleted the java-notice branch August 24, 2023 01:44
@tisonkun
Copy link
Member Author

@raboof Thanks for your information!

I added this NOTICE file following Flink's practice. Correct years is one of the target, the other one is properly mentioning dependencies' NOTICE/LICENSE if any, although opendal-java doesn't have one now.

@tisonkun
Copy link
Member Author

But the timezone problem seems need to resolve as for "project.build.outputTimestamp". Do you have some idea here or may you directly send a patch against this repo to fix it?

@raboof
Copy link
Member

raboof commented Aug 24, 2023

I added this NOTICE file following Flink's practice. Correct years is one of the target, the other one is properly mentioning dependencies' NOTICE/LICENSE if any, although opendal-java doesn't have one now.

👍 great!

But the timezone problem seems need to resolve as for "project.build.outputTimestamp". Do you have some idea here or may you directly send a patch against this repo to fix it?

I don't think there is any timezone issue anymore now that you've replaced the NOTICE. The code in https://github.com/apache/maven-remote-resources-plugin that introduced that problem is not used in the opendal binding project anymore.

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.

NOTICE year is incorrect
4 participants