-
Notifications
You must be signed in to change notification settings - Fork 25
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
Added warning about license change for Oracle Java #5
Conversation
FWIW I would also consider moving the warning hidden in the help menu so it is always visible like this warning as well. |
@@ -47,4 +47,11 @@ THE SOFTWARE. | |||
<f:entry title="" field="acceptLicense"> | |||
<f:checkbox title="${%I agree to the Java SE Development Kit License Agreement}"/> | |||
</f:entry> | |||
<f:entry> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<f:entry>
has semantic meaning in terms of form entry and data binding, so it might be better to leave it out and just have the <div>
, although I don't know how that would look. Do you have a screenshot of what the change looks like? The plugin has more or less no tests, so we'd want to manually check it to make sure everything still works before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using <f:block>
is just plain html and this should not influence functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah, I forgot about <f:block>
. I think it might be better to use that, but I don't feel too strongly. Maybe wait and see what other reviewers think. Thanks for the screenshots!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm, thanks! It might be also ok to reference the AdoptOpenJDK plugin as alternative
From a technical point of view, I haven't any concern. I'm a bit worried about the legal aspect. What happens with the installations already set? Maybe they are for production or commercial use:
|
I opened this PR and jenkins-infra/crawler#82 as a service to the people that will continue to use Oracle JDK and this plugin after Oracle's license change. I'm personally switching to AdoptOpenJDK, that's why I wrote the AdoptOpenJDK installer plugin, so I don't feel strongly about this PR. But if we can be held accountable in any way then I will vote for discontinuing this plugin. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think it worth a breaking change in the plugin. I suggest we introduce a new field instead of acceptLicense
so that the value is reset to false
. The original field can be made transient. Such change will force all users to reaccept the license before using the installer
To highlight such breacking change, we can mark the new plugin version as incompatible: https://wiki.jenkins.io/display/JENKINS/Marking+a+new+plugin+version+as+incompatible+with+older+versions
Bonus points for changing the plugin name to "Oracle JDK Installer": https://issues.jenkins-ci.org/browse/JENKINS-57394
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
requesting changes according to the comments. Main purpose is to get feedback from others
${%Oracle Java SE 11+ is not available for business, commercial or production use without a commercial license.}<br /> | ||
${%Public updates for Oracle Java SE 8 released after January 2019 will not be available for business, commercial or production use without a commercial license.}<br /> | ||
<a href="https://www.oracle.com/technetwork/java/javase/overview/oracle-jdk-faqs.html" target="_blank">${%Oracle Java SE Licensing FAQ}</a><br /> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once #6 is integrated, we can also reference some internal documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After some consideration, I think it is a net benefit to integrate the change as is.
My plan is to create a dev list discussion for a breaking change after it so that we can sign it off in the wider community.
Merging the code as is, https://groups.google.com/forum/#!topic/jenkinsci-dev/GT1IetgfV4o is a follow-up to make decisions about breaking changes |
Thanks @hrmohr ! |
Oracle has changed license for their Java SE and this PR add a warning text and a link to their Oracle Java SE Licensing FAQ.
If Include all Oracle JDKs #82 gets merged and we add support for all Oracle Java versions starting from 1.4 it would be best if the user is informed about Oracle Java licensing.