-
Notifications
You must be signed in to change notification settings - Fork 114
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
introduce ResolveRelativePaths as a distinct operation vs Normalization #414
Conversation
96d3404
to
ea79f1e
Compare
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.
Looks like a small issue on Windows with escapes 🙄 but LGTM! Waaaay better separation of concerns ✨
Signed-off-by: Nicolas De Loof <[email protected]>
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
@@ -185,6 +185,7 @@ func Load(configDetails types.ConfigDetails, options ...func(*Options)) (*types. | |||
LookupValue: configDetails.LookupEnv, | |||
TypeCastMapping: interpolateTypeCastMapping, | |||
}, | |||
ResolvePaths: true, |
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.
@ndeloof this is a change in default behavior right? Before this was false by default.
If I explicitly set it to false through the options argument of the Load
function the behavior would be the same as before this change right?
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.
The 'ResolvePaths' make the unit-test fail in the nerdctl :-)
relate : containerd/nerdctl#2338
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.
@tomfeigin code that has been moved into ResolveRelativePaths
used to be inside Normalize
which is enabled until you explicitly set SkipNormalization
, so this is not a behavior change. Just those two processing aspects have been separated into distinct flags.
Normalization
is used both to define implicit resources and to resolve relative paths, so it brings a parameter so apply load optionResolvePaths
this PR introduces and explicit
ResolveRelativePaths
to manage this option as an explicit load operationthis also allows to avoid duplicating the code when loading service
extends
and resolving basefile-relative paths