-
Notifications
You must be signed in to change notification settings - Fork 323
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
Fix cli to work on windows #1139
Conversation
@@ -86,11 +93,7 @@ func readFile(chart embed.FS, f string, pathPrefix string) (*loader.BufferedFile | |||
if err != nil { | |||
return nil, err | |||
} | |||
// Remove the path prefix. |
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.
Rel()
doesn't exist in the path
package. I looked at copying it from filepath
but it uses a lot of internal stuff to filepath
so not an easy copy. I think we're okay just trimming the prefix here because we're always passing in pathPrefix (hence variable name), not something we need to properly calculate the relative directory of.
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.
This looks really good. Thank you for catching and fixing it.
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.
🙏
Previously would error about missing file `consul\Chart.yaml`. This is because the go embedded filesystem must be accessed using `/` delimiters but we were using `filepath` functions which on windows use `\`.
Previously would error about missing file
consul\Chart.yaml
. Thisis because the go embedded filesystem must be accessed using
/
delimiters but we were using
filepath
functions which on windowsuse
\
.How I've tested this PR:
How I expect reviewers to test this PR:
Checklist: