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

[JENKINS-59383] Fix the DiskMonitorSpaceDescriptor toString() message when the monitor alert is not triggered #4203

Merged
merged 2 commits into from Sep 17, 2019

Conversation

Dohbedoh
Copy link
Contributor

@Dohbedoh Dohbedoh commented Sep 16, 2019

See JENKINS-59383.

Proposed changelog entries

  • [JENKINS-59383] Fix the DiskMonitorSpaceDescriptor toString() message

Submitter checklist

  • JIRA issue is well described
  • Changelog entry appropriate for the audience affected by the change (users or developer, depending on the change). Examples
    * Use the Internal: prefix if the change has no user-visible impact (API, test frameworks, etc.)
  • Appropriate autotests or explanation to why this change has no tests
  • For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@mention @reviewbybees

Copy link
Member

@batmat batmat left a comment

Choose a reason for hiding this comment

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

LGTM, though I wonder how/if we could factor out the if(triggered) logic between toString and toHtml to avoid this to come again in the future.

@@ -71,7 +71,10 @@ public DiskSpace(String path, long size) {

@Override
public String toString() {
return Messages.DiskSpaceMonitorDescriptor_DiskSpace_FreeSpaceTooLow(getGbLeft(), path);
if(triggered) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if(triggered) {
if (triggered) {

Copy link
Contributor

Choose a reason for hiding this comment

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

@Dohbedoh could you please apply this? Obviously not a blocker.

@oleg-nenashev oleg-nenashev added the bug For changelog: Minor bug. Will be listed after features label Sep 16, 2019
@oleg-nenashev oleg-nenashev changed the title [JENKINS-59383] Fix the DiskMonitorSpaceDescriptor toString() message [JENKINS-59383] Fix the DiskMonitorSpaceDescriptor toString() message when the monitor alert is not triggered Sep 16, 2019
Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

Will merge tomorrow if no negative feedback

@oleg-nenashev oleg-nenashev added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Sep 16, 2019
@oleg-nenashev oleg-nenashev merged commit 2bd71b6 into jenkinsci:master Sep 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For changelog: Minor bug. Will be listed after features ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback
Projects
None yet
9 participants