Add plot description #58

ahcorde · 2020-02-27T10:18:03Z

I would like to add a description to the plots. As you can see in the following plot, the idea is to add a small text below the plots.

The text is only shown the first time that I load the site, then the text disappear. any thoughts on this?

Signed-off-by: ahcorde <ahcorde@gmail.com>

codecov · 2020-02-27T11:11:13Z

Codecov Report

Merging #58 into master will decrease coverage by 0.18%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master      #58      +/-   ##
============================================
- Coverage     43.54%   43.35%   -0.19%     
- Complexity      187      224      +37     
============================================
  Files            18       18              
  Lines          1254     1610     +356     
  Branches        193      278      +85     
============================================
+ Hits            546      698     +152     
- Misses          643      843     +200     
- Partials         65       69       +4

Impacted Files	Coverage Δ	Complexity Δ
src/main/java/hudson/plugins/plot/PlotBuilder.java	`49.65% <0.00%> (-12.51%)`	`29.00% <0.00%> (+7.00%)`	⬇️
src/main/java/hudson/plugins/plot/CSVSeries.java	`60.50% <0.00%> (+0.38%)`	`31.00% <0.00%> (+2.00%)`
.../java/hudson/plugins/plot/MatrixPlotPublisher.java	`57.54% <0.00%> (+0.40%)`	`18.00% <0.00%> (+2.00%)`
src/main/java/hudson/plugins/plot/PlotReport.java	`51.11% <0.00%> (+4.11%)`	`11.00% <0.00%> (+1.00%)`
src/main/java/hudson/plugins/plot/Plot.java	`31.50% <0.00%> (+5.54%)`	`55.00% <0.00%> (+25.00%)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update efbbacb...7d8730c. Read the comment docs.

vgaidarji

@ahcorde I would rename Description title field to Plot description and move it below Plot title in jelly file.

It would be nice to follow one of the PR requirements -> Git commits follow best practices.

vgaidarji · 2020-02-29T11:28:46Z

src/main/java/hudson/plugins/plot/Plot.java

@@ -176,6 +176,12 @@
    @SuppressWarnings("visibilitymodifier")
    public String title;

+    /**
+     * Description .


Please add a more descriptive java doc.

vgaidarji · 2020-02-29T11:30:54Z

src/main/java/hudson/plugins/plot/Plot.java

        this.title = title;
        this.yaxis = yaxis;
        this.group = group;
        this.numBuilds = numBuilds;
+        this.description = description;


Please follow parameters order in CTOR and assignments order inside CTOR -> move below yaxisMaximum.

vgaidarji · 2020-02-29T11:33:32Z

src/main/java/hudson/plugins/plot/Plot.java

@@ -420,7 +427,8 @@ public String toString() {
                + getRightBuildNum() + "),HASLEGEND(" + hasLegend()
                + "),ISLOGARITHMIC(" + isLogarithmic() + "),YAXISMINIMUM("
                + yaxisMinimum + "),YAXISMAXIMUM(" + yaxisMaximum
-                + "),FILENAME(" + getCsvFileName() + ")";
+                + "),FILENAME(" + getCsvFileName() + "),DESCRIPTION"
+                + getDescription() + ")";


Missing opening "(" before getDescription().

vgaidarji · 2020-02-29T11:39:17Z

src/main/java/hudson/plugins/plot/PlotBuilder.java

@@ -97,6 +99,11 @@ public final void setTitle(@CheckForNull String title) {
        this.title = Util.fixEmptyAndTrim(title);
    }

+    @DataBoundSetter
+    public final void setDescription(@CheckForNull String title) {


I would move it below setYaxisMaximum to preserve the order since description is the last parameter and is after yaxisMaximum parameter.

title -> description

vgaidarji · 2020-02-29T11:56:40Z

src/main/resources/hudson/plugins/plot/PlotReport/index.jelly

@@ -51,6 +51,9 @@
          </div>
        </j:if>
        <img src="getPlot?index=${index}&amp;width=750&amp;height=450" width="750" height="450" lazymap="getPlotMap?index=${index}" />
+        <div>
+          <h4>Description</h4>: ${plot.description}


In case no description provided, the empty description is shown

Since the intent was to make this plot parameter optional, we shouldn't show it if the field is empty.

Maybe make the text bold using <b> tag instead of <h4>? This way you can keep text in one line.

Proper way of retrieving the description text would be ${it.getPlotDescription(index)} and adding following method to PlotReport.java

// called from PlotReport/index.jelly public String getPlotDescription(int i) { Plot plot = getPlot(i); return plot.getDescription(); }

vgaidarji · 2020-02-29T12:12:36Z

src/main/resources/hudson/plugins/plot/PlotReport/index.jelly

@@ -51,6 +51,9 @@
          </div>
        </j:if>
        <img src="getPlot?index=${index}&amp;width=750&amp;height=450" width="750" height="450" lazymap="getPlotMap?index=${index}" />
+        <div>


I would add style="width:750px" to description div to align its max width with plot image.

<div style="width:750px">

This will also solve the issue of very long text which will go to the next line after max width is reached 👍

vgaidarji · 2020-02-29T14:23:17Z

src/main/java/hudson/plugins/plot/Plot.java

@@ -637,6 +658,7 @@ public void plotGraph(StaplerRequest req, StaplerResponse rsp)
        setTitle(req);
        setStyle(req);
        setUseDescr(req);
+        setDescription(req);


This will remove description set, please remove.

vgaidarji · 2020-02-29T14:23:19Z

src/main/java/hudson/plugins/plot/Plot.java

@@ -665,6 +687,7 @@ public void plotGraphMap(StaplerRequest req, StaplerResponse rsp) throws IOExcep
        setTitle(req);
        setStyle(req);
        setUseDescr(req);
+        setDescription(req);


This will remove description set, please remove.

vgaidarji · 2020-02-29T14:31:44Z

src/main/webapp/help-description.html

@@ -0,0 +1,3 @@
+<div>
+Optional.  Specifies the plot description.


Double-space between sentences.

vgaidarji

@ahcorde I still think having plot group, title, description together creates better visual organization, and please rename Description title to Plot description.

Everything else works just fine 👍

src/main/resources/hudson/plugins/plot/PlotReport/index.jelly

ahcorde · 2020-03-09T08:56:01Z

Thank you for the reviews!

vgaidarji · 2020-03-09T08:56:40Z

@ahcorde welcome. Thanks for your contribution 🎉 Will release the new plugin version soon.

vgaidarji · 2020-03-09T09:08:08Z

@ahcorde 2.1.5 version (see it in repo.jenkins-ci.org) was published which includes changes from current PR. The new plugin version should appear in plugins update center in the matter of hours.
Thanks a lot!

added description to each one of the plots

2f826e7

Signed-off-by: ahcorde <ahcorde@gmail.com>

ahcorde force-pushed the ahcorde/feature/description branch from 1a7e9d0 to 2f826e7 Compare February 27, 2020 10:26

vgaidarji requested changes Feb 29, 2020

View reviewed changes

ahcorde added 2 commits February 29, 2020 17:43

Includind feedback

d233009

allowed expressions to produce mark up

ac1d4d5

ahcorde requested a review from vgaidarji March 4, 2020 07:51

vgaidarji requested changes Mar 7, 2020

View reviewed changes

src/main/resources/hudson/plugins/plot/PlotReport/index.jelly Show resolved Hide resolved

vgaidarji self-assigned this Mar 7, 2020

ahcorde added 2 commits March 7, 2020 14:27

fixed name and moved plot description in jelly files

88f34c1

Added empty check in plot description

7d8730c

ahcorde force-pushed the ahcorde/feature/description branch from 0878019 to 7d8730c Compare March 7, 2020 13:39

vgaidarji approved these changes Mar 9, 2020

View reviewed changes

vgaidarji changed the title ~~Added description to plots~~ Add plot description Mar 9, 2020

vgaidarji merged commit e26dfc0 into jenkinsci:master Mar 9, 2020

vgaidarji added the feature New feature or improvement label Mar 18, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add plot description #58

Add plot description #58

ahcorde commented Feb 27, 2020

codecov bot commented Feb 27, 2020 •

edited

Loading

vgaidarji left a comment •

edited

Loading

vgaidarji Feb 29, 2020

vgaidarji Feb 29, 2020

vgaidarji Feb 29, 2020

vgaidarji Feb 29, 2020

vgaidarji Feb 29, 2020

vgaidarji Feb 29, 2020

vgaidarji Feb 29, 2020

vgaidarji Feb 29, 2020

vgaidarji Feb 29, 2020

vgaidarji Feb 29, 2020

vgaidarji Feb 29, 2020

vgaidarji Feb 29, 2020

vgaidarji Feb 29, 2020

vgaidarji left a comment •

edited

Loading

ahcorde commented Mar 9, 2020

vgaidarji commented Mar 9, 2020

vgaidarji commented Mar 9, 2020 •

edited

Loading

		@@ -0,0 +1,3 @@
		<div>
		Optional. Specifies the plot description.

Add plot description #58

Add plot description #58

Conversation

ahcorde commented Feb 27, 2020

codecov bot commented Feb 27, 2020 • edited Loading

Codecov Report

vgaidarji left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vgaidarji left a comment • edited Loading

Choose a reason for hiding this comment

ahcorde commented Mar 9, 2020

vgaidarji commented Mar 9, 2020

vgaidarji commented Mar 9, 2020 • edited Loading

codecov bot commented Feb 27, 2020 •

edited

Loading

vgaidarji left a comment •

edited

Loading

vgaidarji left a comment •

edited

Loading

vgaidarji commented Mar 9, 2020 •

edited

Loading