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

Add JiraIssuesSizeEnvironmentVariableBuilder build step #119

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mupakoz
Copy link

@mupakoz mupakoz commented Jan 10, 2017

Adds JiraIssuesSizeEnvironmentVariableBuilder build step which enables saving number of issues matching a JQL query in a environment variable of specified name.

I came up with this idea for a new feature when I wanted to fail the build in case there are issues on JIRA that were resolved but still untested. Thanks to the plugin we can save the number of issues we are interested in and then use it in the next steps (for example a shell script or a conditional step).

Please let me know what you think!


This change is Reviewable

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 48.248% when pulling 97a111c on mupakoz:master into 486879b on jenkinsci:master.

@rantoniuk
Copy link
Contributor

How about extending JiraEnvironmentContributingAction + JiraEnvironmentVariableBuilder to provide issue number in ENV from the used AbstractIssueSelector?

@mupakoz
Copy link
Author

mupakoz commented Jan 10, 2017

It seems that there is a problem with the JiraEnvironmentVariableBuilder if I understand the files correctly (forgive me I am a Jenkins plugin developer newbie). In the config.jelly there is a condition for the issue selector: 'if descriptor.hasIssueSelectors()' but the method 'hasIssueSelectors' is not defined in the class DescriptorImpl. That's why the control for choosing the selector does not show up. I can fix the error in my PR.

Do you think the ENV var name should be set arbitrary or the user should be able to set it?

@mupakoz
Copy link
Author

mupakoz commented Jan 13, 2017

I just thought that when using JiraEnvironmentVariableBuilder with arbitrary variable names we would be limited to only one set of issues per job.

@rantoniuk
Copy link
Contributor

  1. see https://github.com/jenkinsci/jira-plugin/blob/master/src/main/java/hudson/plugins/jira/JiraEnvironmentContributingAction.java#L14
    I would use JIRA_ISSUES_SIZE

  2. Yes, the argument for using it multiple times is a valid one. I'm curious what's your use case for this?

@mupakoz
Copy link
Author

mupakoz commented Jan 16, 2017

  1. My proposal is to use JIRA_ISSUES_SIZE by default but to put a text input in the 'advanced' section: so it would be configurable.
  2. In my use case I only need one number, but I can imagine a scenario where two lists of issues would be needed, but we could leave that until a real feature request will come.

@rantoniuk
Copy link
Contributor

  1. agreed, if you want to code it then I'm fine :)
  2. 👍

*/
package hudson.plugins.jira;

import hudson.*;
Copy link
Contributor

Choose a reason for hiding this comment

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

We have agreed approach not to use wildcard imports, could you expand those?

}

@Override
public void buildEnvVars(AbstractBuild<?, ?> ab, EnvVars ev) {
Copy link
Contributor

Choose a reason for hiding this comment

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

no two letter variable names please :)

@rantoniuk rantoniuk added this to the 2.4 milestone Feb 4, 2017
@rantoniuk
Copy link
Contributor

@mupakoz ping, are you able to address review comments?

@mupakoz
Copy link
Author

mupakoz commented Mar 1, 2017

Yes, sorry for my inactivity. I will fix the implementation and the comments by the end of this week.

@rantoniuk
Copy link
Contributor

@mupakoz ping? :-)

@mupakoz mupakoz force-pushed the master branch 2 times, most recently from cba77e6 to 0f1eeb3 Compare March 10, 2017 14:33
@mupakoz
Copy link
Author

mupakoz commented Mar 10, 2017

@warden done. Let me know what you think.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 48.075% when pulling 0f1eeb3 on mupakoz:master into 45e79b3 on jenkinsci:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 48.024% when pulling 0f1eeb3 on mupakoz:master into 45e79b3 on jenkinsci:master.

@mupakoz
Copy link
Author

mupakoz commented Mar 10, 2017

I also added the hasIssueSelectors() method because I think it was missing in the class.

</j:if>
<f:advanced>
<f:entry title="${%Issues size variable name}" field="issuesSizeVariableName">
<f:textbox default="JIRA_ISSUES_SIZE"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

better to use word COUNT instead of SIZE.
size is usually used when creating an object of specific size to show how many elements can fit in collection, not how many there are

Copy link
Contributor

Choose a reason for hiding this comment

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

Applies to all files of course :)

@rantoniuk rantoniuk removed this from the 2.4 milestone Apr 30, 2019
@rantoniuk rantoniuk added the needs-rebase Waiting for contributor action label Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-rebase Waiting for contributor action
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants