Refactor tmpfsMounts() to process tmpfs path correctly - #2103
Conversation
jglogan
left a comment
jglogan
left a comment
There was a problem hiding this comment.
@LevelVoid Don't forget the absolute path test, see comment.
Also, you'll need to configure your account for verified commit signatures or we can't merge, see: https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification#about-commit-signature-verification.
Thank you!
| throw ContainerizationError(.invalidArgument, message: "mount destination cannot be empty") | ||
| } | ||
|
|
||
| let normalizedDest = FilePath(destination).lexicallyNormalized().string |
There was a problem hiding this comment.
compute the filepath first and guard to ensure that isAbsolute is true.
Then do the normalization and continue.
Add a test to ensure non-absolute destinations fail.
There was a problem hiding this comment.
compute the filepath first and guard to ensure that
isAbsoluteis true. Then do the normalization and continue.Add a test to ensure non-absolute destinations fail.
Got it, i will do that and push after configuring my account.
|
@LevelVoid Still seeing some formatting issues: ❌ The following files require formatting or license header updates:
Tests/ContainerAPIClientTests/ParserTest.swift |
@jglogan I fixed the issues causing failure, and it's ready to review. |
jglogan
left a comment
jglogan
left a comment
There was a problem hiding this comment.
swift format is quite particular, two spaces
|
@LevelVoid I touched up the formatting. That means I'll need to have someone else approve the PR tomorrow to get it merged, but everything looks good to go. Thank you for fixing this! |
Code Coverage
|
|
Thanks! Appreciate you taking the time to review it. |
3 participants
Type of Change
Motivation and Context
Fixes #2094 : tmpfs works as stated in issue with tests added
Testing