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

Makes WorkflowRun.checkouts() public #130

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@mikeyeung123
Copy link

commented May 16, 2019

Allows plugins to access the SCMs in the current pipeline run. Currently the next best option is WorkflowJob.getSCMs(), which gets the SCMs of the previous run.

@mikeyeung123

This comment has been minimized.

Copy link
Author

commented May 16, 2019

I don't think the build error is because of the PR...

@dwnusbaum
Copy link
Member

left a comment

I'm not sure how this method is normally used, but exposing something similar to it seems ok. Do you have a corresponding update in some other plugin that would use this method? If so, please link it to this PR.

@@ -758,7 +758,8 @@ public boolean hasntStartedYet() {
return isBuilding(); // there is no equivalent to a post-production state for flows
}

synchronized @Nonnull List<SCMCheckout> checkouts(@CheckForNull TaskListener listener) {
@Exported

This comment has been minimized.

Copy link
@dwnusbaum

dwnusbaum Jun 18, 2019

Member

Does @Exported even work for a method that takes parameters?

@@ -758,7 +758,8 @@ public boolean hasntStartedYet() {
return isBuilding(); // there is no equivalent to a post-production state for flows
}

synchronized @Nonnull List<SCMCheckout> checkouts(@CheckForNull TaskListener listener) {
@Exported
public synchronized @Nonnull List<SCMCheckout> checkouts(@CheckForNull TaskListener listener) {

This comment has been minimized.

Copy link
@dwnusbaum

dwnusbaum Jun 18, 2019

Member

Since SCMCheckout is a package-private class, other plugins wouldn't be able to use this method without making that type public as well (You'd also need to add getters for its fields, etc.). I think it would be better to have a new method that returns List<SCM> (or maybe List<? extends SCM>, not sure what is normally used for this kind of thing) instead and populates that list using WorkflowRun.checkouts internally.

@dwnusbaum

This comment has been minimized.

Copy link
Member

commented Jun 18, 2019

I don't think the build error is because of the PR...

You're right, it's not related, and it was fixed by updating to version 3.43 of the parent pom: 76d44d5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.