Improve handling of file names and paths#144
Open
enpaul wants to merge 4 commits intocontainerbuildsystem:masterfrom
Open
Improve handling of file names and paths#144enpaul wants to merge 4 commits intocontainerbuildsystem:masterfrom
enpaul wants to merge 4 commits intocontainerbuildsystem:masterfrom
Conversation
chmeliik
reviewed
Oct 2, 2023
Contributor
chmeliik
left a comment
chmeliik
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
Looks good to me, just one backwards compatibility consideration and some unittest nitpicks
chmeliik
approved these changes
Oct 3, 2023
Contributor
chmeliik
left a comment
chmeliik
left a comment
There was a problem hiding this comment.
LGTM except 1 linter error
the pylint and markdownlint errors are unrelated
Add new attribute 'dockerfile' to the DockerfileParser class which exposes the path to the dockerfile as a PathLib object Add new property 'dockerfile_path' to the DockerfileParser class which exposes the 'dockerfile' attribute as a string for backwards compatibility Signed-off-by: Ethan Paul <ethan.paul@portalinstruments.com>
Signed-off-by: Ethan Paul <ethan.paul@portalinstruments.com>
Signed-off-by: Ethan Paul <ethan.paul@portalinstruments.com>
Signed-off-by: Ethan Paul <ethan.paul@portalinstruments.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thank you so much for making this project! Each commit in this PR is self contained and could be reviewed separately, let me know if there's anything I can do to make the review process easier.
Summary of changes:
DockerfileParserclass to usepathlib.Pathobjects for file read/write operationsDockerfileParser.dockerfileattribute to expose the path to the Dockerfile as apathlib.Pathobject while maintaining backwards compatibility with the existingDockerfileParser.dockerfile_pathproperty as a stringdockerfile_filenamekeyword toDockerfileParser.__init__()to allow users to provide an alternative name for their dockerfile (ex:Dockerfile.dev,Dockerfile.sample,Containerfile, etc) while maintaining backwards compatibility with the existing default ofDockerfileMaintainers will complete the following section