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.
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
Cache location config #120
Cache location config #120
Changes from 3 commits
667b8be
d802554
90552da
2b20b41
1613845
d0c6e33
c12e0c2
3486868
37e2445
fffb45a
38af01c
c067775
8d74f67
6cb9b2c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Don't check the dir first; just create it and inspect the
mkdir
return code (if it'sEEXIST
, then the directory already exists and move on).For error message creation, it would be better to have an output variable that's the error message so you can say precisely the file operation that failed. The caller of this function currently has something generic that won't be useful for debugging.
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.
What happens if someone provides a path of
///foo//bar
? May need anif (!component.empty())
here to avoid inserting unnecessary items.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.
You caught my sinful shortcut. Since ultimately this output only gets passed to mkdir, the POSIX standard should suppress the extra slashes (so that the cache would be dumped at
/foo/bar
even if it was specified as///foo//bar
). But I agree, I should add the check in case this function ever gets used in another spot where someone might not know about that peculiarity.