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
[JENKINS-66177] Ability to disable Java 11 monitor #5628
Conversation
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.
This seems reasonably well-limited scope.
@@ -46,9 +47,11 @@ | |||
@Symbol("javaVersionRecommendation") | |||
public class JavaVersionRecommendationAdminMonitor extends AdministrativeMonitor { | |||
|
|||
private static Boolean disabled = SystemProperties.getBoolean(JavaVersionRecommendationAdminMonitor.class.getName() + ".disabled", false); |
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.
PR to jenkins.io to add to docs?
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.
Given the approvals, and the fact Tim was the submitter of the Java 11 admin monitor PR in the first place, I am moving this forward and marking this PR as We will hence merge this in 24 hours if nobody objects in the meantime. Thank you |
I seem to recall disabling previous administrative monitors in the UI and/or Groovy initialization scripts rather than with system properties. If that is indeed a precedent, it seems that it would be ideal to do it that way for a more consistent and polished user experience. However, I don't feel strongly about this. I don't intend to rain on the parade, and this comment is non-blocking. |
That's for users to disable, I assume this is for distributors. i.e. CloudBees |
Fair enough, but in that case they should say so explicitly. Nothing in the PR description or the Jira issue indicates that this is meant to be consumed by OEM distributors rather than end users. |
Yes, this is mainly for distributors. However I can also think about other usages, ie. company X uses a plugin (which might be home made) which is not Java 11 compatible yet, so they don't want Jenkins administrators to even see this monitor to avoid confusion. A sysadmin can set this property at upgrade time and move on. |
they can also use the standard supported approach which is the XML file / jcasc. |
(cherry picked from commit 0b0085d)
[JENKINS-66177] Ability to disable Java 11 monitor (jenkinsci#5628)
See JENKINS-66177.
Proposed changelog entries
Proposed upgrade guidelines
N/A
Submitter checklist
Proposed changelog entries
section only if there are breaking changes or other changes which may require extra steps from users during the upgradeDesired reviewers
@mention
Maintainer checklist
Before the changes are marked as
ready-for-merge
:Proposed changelog entries
are correctupgrade-guide-needed
label is set and there is aProposed upgrade guidelines
section in the PR title. (example)lts-candidate
to be considered (see query).