JENKINS-28600: Support Dockerfiles with non-standard names. #23
Conversation
|
Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests |
| private boolean buildAndTag() throws MacroEvaluationException, IOException, InterruptedException { | ||
| String context = defined(getDockerfilePath()) ? | ||
| getDockerfilePath() : "."; | ||
| File context = new File(defined(getBuildContext()) ? getBuildContext() : "."); |
There was a problem hiding this comment.
Should use FilePath and default to the workspace IMO
|
Seems this behavior will be a breaking change for builds using absolute paths. It needs to be updated |
|
@johndistasio any ideas? |
|
@zeeraw Yeah, I plan on addressing the concerns when I get more than a few minutes to work on this stuff. |
* Allow absolute paths for dockerfilePath. * Use Jenkins' FilePath for file operations. * Default build context to workspace.
|
@oleg-nenashev I made some changes to address the issues you pointed out, please take a look when you get a chance. |
|
@johndistasio There's only one minor comment left (regarding the input data). It would be also useful to add the new field to roundtrip unit tests. |
|
Added the requested input cleanup. I'll look into adding the round-trip testing. |
| String context = defined(getDockerfilePath()) ? | ||
| getDockerfilePath() : "."; | ||
| FilePath context = defined(getBuildContext()) ? | ||
| new FilePath(new File(getBuildContext())) : build.getWorkspace(); |
There was a problem hiding this comment.
minor: spaces instead of tabs
|
Is this ever going to be merged in? |
|
👍 from me, but I'm not the plugin owner |
|
LGTM (Carlos knows code more recently)
|
JENKINS-28600: Support Dockerfiles with non-standard names.
|
sorry for the delay |
Hello,
This pull request addresses JENKINS-28600 by changing the build process to work a little more like it does with the Docker client.