icfnext / cq-component-maven-plugin

Other
22 stars 35 forks source link

Provide Configurable Package Name #56

Closed christopher-w-leggett closed 7 years ago

christopher-w-leggett commented 7 years ago

Looking to add ability to specify package names.

michaelhodgdon commented 7 years ago

Chris, what is the use case for configuring the temp package name? Can't we just derive that from the configured package name?

christopher-w-leggett commented 7 years ago

We can derive it from the configured package name. I don't have a use case for making it fully configurable. I'll work on making the change.

christopher-w-leggett commented 7 years ago

Updated. Let me know if you have any concerns.

michaelhodgdon commented 7 years ago

That checking for the zip ending bit made me wonder, why are we including that in the package filename? Shouldn't we just have the file name and add zip ourselves (so the default would be ${project.build.directory}/${project.build.finalName} and we'd add the extension when we need to). There isn't a use case where this isn't a zip.

christopher-w-leggett commented 7 years ago

That makes sense. I made the appropriate changes.

michaelhodgdon commented 7 years ago

One last thing, why is the build directory part of the file name configuration?

christopher-w-leggett commented 7 years ago

I modeled it after other plugins (ex. aem-package-maven-plugin) for consistency, but we are slowly moving in another direction (extension removal). I don't see any reason why it would be required.

michaelhodgdon commented 7 years ago

Yeah I don't think it should be. All artifacts should be in the build directory. This should be the last change and I'll merge it in.

christopher-w-leggett commented 7 years ago

Build directory has been removed from the configuration.