-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Added ability to run Dockerfile.SUFFIX ContainerAction #1738
Added ability to run Dockerfile.SUFFIX ContainerAction #1738
Conversation
@thboop Do you think we should wrap this one in a FF? |
|
||
public static bool IsDockerfile(string image) | ||
{ | ||
if (image.StartsWith("docker://", StringComparison.OrdinalIgnoreCase)) |
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.
Let's keep this if, depending on how IsDockerFile
is called, we may end up checking if the image starts with docker://
multiple times.
But as a util method, I think IsDockerFile
should also do the check itself so it's not reliant on being called in an if/else that has checked docker://
already.
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.
lgtm
* Added ability to run Dockerfile.SUFFIX ContainerAction * Extracted IsDockerFile method * reformatted, moved from index to Last() * extracted IsDockerfile to DockerUtil with L0 * added check for IsDockerfile to account for docker:// * updated test to clearly show path/dockerfile:tag * fail if Data.Image is not Dockerfile or docker://[image]
* Revert "Added ability to run Dockerfile.SUFFIX ContainerAction (actions#1738)" 20b7e86 * 291.1 release notes
Closes #1416
This is an enhancement to the runner allowing it to run Dockerfile.SUFFIX files, which is allowed by Docker, but doesn't work in GitHub Actions