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-40448: add to existing actions for a run #97

Merged
merged 3 commits into from Jan 11, 2017

Conversation

juliantcook
Copy link

Please let me know if this is a viable request.

@Jimilian
Copy link

Sorry for delayed answer, I will double-check how same issue (multiple "actions" for 1 run) is handled by "official" plugins. I see point in your solution, but I expect (~hope) that Cloudbees will add some native support for stages in UI. In this case current solution looks like workaround.

Copy link

@Jimilian Jimilian left a comment

Choose a reason for hiding this comment

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

Everything is ok in general. But, please, address this one tiny remark ;)

@@ -66,6 +66,10 @@ public String getProfile() {
return artifacts;
}

public void addArtifacts(List<FingerprintRecord> artifacts) {

Choose a reason for hiding this comment

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

This method is not needed. I'm not a Jenkins expert, but usually I'm trying to avoid creation of new public methods :)

You can use getArtifacts().addAll(). It should work as well.

@juliantcook
Copy link
Author

Whoops I wasn't meant to add the test on this branch... I have the flu, wasn't sure if that was the correct place for the test.

@juliantcook
Copy link
Author

@Jimilian Let me know if the test is wrong and I can revert it for now.

@Jimilian Jimilian merged commit e7cc2e4 into jenkinsci:master Jan 11, 2017
@Jimilian
Copy link

@juliantcook Sorry, I didn't receive any notification about your changes. Looks good for me. I will try to find some time to make this plugin more unit-test friendly :)


HtmlPage page = j.createWebClient().goTo("configure");
WebAssert.assertTextPresent(page, "S3 profile random name");
}

@Test
public void multiplePublishersUseExistingActions() throws Exception {

Choose a reason for hiding this comment

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

It's not best place for this test, but let's keep it as it is :)

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