-
Notifications
You must be signed in to change notification settings - Fork 129
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-41854] Recontextualizing FilePath on demand #86
[JENKINS-41854] Recontextualizing FilePath on demand #86
Conversation
…n …` message, rather than the actual CWD.
…o make PushdStepTest pass.
…tep should never have been created to begin with.
On hold as in jenkinsci/workflow-support-plugin#95 (comment). |
@@ -90,7 +91,8 @@ public String getPath() { | |||
FilePath dir = getContext().get(FilePath.class).child(path); | |||
getContext().get(TaskListener.class).getLogger().println("Running in " + dir); | |||
getContext().newBodyInvoker() | |||
.withContext(dir) | |||
// TODO perhaps should just be moved into workflow-durable-task-step? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 from me, I think it would make sense to keep all core steps that manipulate FilePath
in the same plugin. Want to do that now or just leave it as a TODO?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all core steps that manipulate
FilePath
There are many other steps in this plugin which consume it, including for example PwdStep
which you could see as the counterpart to PushdStep
. This just happens to be the only one in this plugin which binds it. So, not sure there is a benefit, other than avoiding the need for FilePathDynamicContext
to be used across plugin boundaries. No strong feeling at this point.
Downstream of jenkinsci/workflow-durable-task-step-plugin#101.