-
Notifications
You must be signed in to change notification settings - Fork 118
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
Add fs.FS support to deploy, inspect, diff #799
Conversation
Use struct fields as flag defaults. If the flag structs are being filled in programmatically, the values are cleared during flag parsing if the flag defaults differ from the current struct values, which is undesirable. Signed-off-by: Andy Goldstein <andy.goldstein@redhat.com>
Add fs.FS support to LocalFileSource, FileResource, and the commands that use them (deploy, diff, inspect). This is useful so these commands can be invoked programmatically. Signed-off-by: Andy Goldstein <andy.goldstein@redhat.com>
I am going to take a closer look at this in my daytime tomorrow. I was wondering if you could create an issue illustrating how this change helps and elaborating on your use case? That context would definitely be helpful! |
I created #800 for this, thanks! |
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.
Thank you so much for the PR @ncdc ❤️
Overall it's looking really good (just left some minor comments).
Signed-off-by: Andy Goldstein <andy.goldstein@redhat.com>
Signed-off-by: Andy Goldstein <andy.goldstein@redhat.com>
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.
@100mik Let me know if you are planning to take a look at it.
Thanks for the PR again! |
What this PR does / why we need it:
Allow deploy, inspect, and diff to use an
fs.FS
for the filesystem so they can be invoked programmatically with filesystems other than just the normal OS one.Which issue(s) this PR fixes:
Fixes #800
Does this PR introduce a user-facing change?
Additional Notes for your reviewer:
Review Checklist:
a link to that PR
change
Additional documentation e.g., Proposal, usage docs, etc.: