-
Notifications
You must be signed in to change notification settings - Fork 442
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 dockerPackageMappings to be absolute paths #1305
base: main
Are you sure you want to change the base?
Conversation
At least one pull request committer is not linked to a user. See https://help.github.com/en/articles/why-are-my-commits-linked-to-the-wrong-user#commits-are-not-linked-to-any-user |
d8dabba
to
38a70df
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.
Thanks for the bug report and the pull request to fix this 🤗
There are a couple of things that make me hesitate to merge this
- the issues seems to be in the MappingsHelper if the paths that are returned are not correct
- using
Paths
seems like a good workaround, but I'm not sure if this is a maintainable solution as this relies heavily on the underlying OS (e.g. file separators, how path representation works ) - a regression test that demonstrates the fix would always be nice
I don't think Your concerne about the underlying OS would be an issue if we are using a different OS in the docker image we are building (not the host machine that is running SBT). I will add a test for it. |
Hi @joan38 thanks again for tackeling this 😃 |
Hi @muuki88, I'm not sure I'll have much time to do this also since we moved away from SBT to Mill. |
Thanks for the update @joan38 😊 |
Fixes #1304