-
Notifications
You must be signed in to change notification settings - Fork 4
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
#207 Refactor containerize-dependencies Mojo to use Apache Velocity for creating dockerfiles #304
Conversation
@@ -34,6 +32,7 @@ | |||
public class ContainerizeDepsMojo extends AbstractHabushuMojo { | |||
|
|||
private static final Logger logger = LoggerFactory.getLogger(ContainerizeDepsMojo.class); | |||
private static final String DOCKERFILE_POETRY_VM = "templates/dockerfile.vm"; |
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.
S: I'd recommend making this a maven-enabled parameter with this as a default. This would allow it to be overridden if appropriate.
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.
templates/dockerfile.vm
lives in the classpath so I'm not sure what use this would have to a downstream developer.
Would we want to give the dev the option of using their own template / changing the existing template? If so, I can expand the velocity engine setup to look in the downstream's file system for a custom dockerfile template file first before deferring to the default in the habushu jar, then make additional documentation for how to do this in the README. Although I imagine someone with extensive customizations would instead disable the dockerfile generation portion before going willy-nilly on it with edits.
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.
I don't think we need to default to looking on the file system first. The idea is to support extension I think. In which case it'd be pretty simple to get the custom template on Habushu's classpath, or change the Velocity configuration to add a new resource loader for template resolution.
@@ -0,0 +1,6 @@ | |||
package org.technologybrewery.habushu; |
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.
Q/S: Instead of creating this new class, could we call PackageManager
in its place?
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.
Good catch! Yeah there is a way we can convert the existing enumerator to strings.
e7a0464
to
a57a5ce
Compare
#207
Refactor containerize-dependencies Mojo to use Apache Velocity for creating dockerfiles
Contains some small test refactoring in prep for #303
Refactor containerize-dependencies Mojo to support UV as a package manager option