Add support for Workflows when loading comment files #682
Conversation
|
Awesome, this is exactly what I was looking for. How do we upvote this? |
src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbCommentFile.java
Show resolved
Hide resolved
| final FilePath workspace = ((Build<?, ?>) build).getWorkspace(); | ||
| final FilePath path = workspace.child(scriptFilePathResolved); | ||
| FilePath workspace; | ||
| FilePath path; |
There was a problem hiding this comment.
looks like these don't need to be declared in the outerscope. it's used within each if block anyway. workspace isn't even used in your if block.
| @@ -195,6 +195,25 @@ job('downstreamJob') { | |||
| } | |||
| ``` | |||
|
|
|||
| ### Pipeline support | |||
There was a problem hiding this comment.
actually, in the above dsl sample code there is no way to use the comment file. do you know the syntax for this?
There was a problem hiding this comment.
Yeah, it'd be really nice to have this.
|
My fixes appear to have removed the previous approval, could I get a re-review on this please? |
|
@bjoernhaeuser @samrocketman is this held up because tests are failing? |
|
@Kentalot we generally don’t merge changes which are failing. Let’s say we merge and the change continues to fail, all proposals will fail from that point on. We like to avoid this so do not merge code which fails as a matter of policy. |
|
Yes, I totally understand why you would not merge. I was just wondering if that was the main reason for the hold up. I will probably look into updating this to see if i can fix this and open a new PR then. |
|
I'm not seeing any tests failing locally fwiw, it looks like the Jenkins test is being sent a signal that interrupts it before it finishes: The tests do not finish as |
|
Then perhaps the tests just need to be retriggered/rerun? |
|
Unfortunately, I don’t have the ability to retrigger tests. |
|
I guess maybe @nosmo can close and reopen or maybe open a new PR? |
|
Closed and reopened as #720 |
When a job is initiated via the pipeline plugin, the Run object is no longer an instance of Build, it becomes an instance of WorkflowRun. This means that the current logic for finding comment files on remote workers does not find the comment file when using a pipeline. Additionally, WorkflowRuns are more complex objects and they can have multiple associated workspaces.
This PR introduces support for finding the files on the remote workspaces.
This change does not allow for seamless discovery of the files in workspaces if multiple nodes are used, and users will be required to
stashandunstashas is appropriate if using remote nodes, but I've included a documentation note to explain this.