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

[FIXED JENKINS-181929] Added ability to push labels with page data. #4

Closed
wants to merge 2 commits into from

Conversation

ribrewguy
Copy link
Contributor

No description provided.

@buildhive
Copy link

Jenkins » confluence-publisher-plugin #15 FAILURE
Looks like there's a problem with this pull request
(what's this?)


// Add the page labels
String labels = this.labels;
if (labels != null && labels.trim().length() > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

FYI, there is a StringUtils.isEmpty(String) method (being used elsewhere in this file) that simplifies this condition. I don't believe isEmpty() will check the trimmed length, but there is a similar StringUtils.isBlank(String) that "Checks if a String is whitespace, empty ("") or null."

@jhansche
Copy link
Member

Looks good, thanks! Since this is a V2 API method, it would be helpful to add a check that the remote server is a V2 server, and maybe hide the option if it's not, or log a warning at runtime to indicate that labels are not supported. The end result would be something like this in addLabels():

if (this.serviceV2 == null) {
    throw new OperationNotSupportedException("Labels are supported as of Confluence v4 and later.");
}

then catch ONSE when the publisher is trying to apply the labels, and append a log warning to indicate to the user that their version of Confluence does not support labels (instead of getting a non-descript NullPointerException when trying to dereference this.serviceV2, which does not explain to the user what actually went wrong).

This plugin should remain compatible with Confluence version 3 (you can test the plugin against the public Jenkins Confluence server if you want -- that server is running v3, and will fail as described above if you try to add labels). Just create a page in your own user space there and set up your dev instance to hit that server.

@jenkinsadmin
Copy link
Member

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

@ribrewguy
Copy link
Contributor Author

Sounds good. I will update the code and resubmit.

@ribrewguy ribrewguy closed this Jul 22, 2013
@buildhive
Copy link

Jenkins » confluence-publisher-plugin #18 SUCCESS
This pull request looks good
(what's this?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants