Repository navigation
Conversation
|
this PR as well as the #603 stem from the same effort I am having all since years, that is to be able to bootstrap the main things in maven eco-system without build cycles. Being able to reliably run modello generations without maven is one of those things. |
40dfef5 to
1d36e0b
Compare
ce34e2d to
3a59e90
Compare
|
Any chance this could be integrated? With the upcoming maven 3.10.0 release, the aliases will be needed and -- if it is possible -- I would prefer not to carry over a huge patch. |
|
Even in this one, I went through the mojo's behaviour once again and tried to align as much as I could. |
|
When the CLI changes enter, I will quickly rebase it to use the ModelloParameterConstants instead of raw strings |
gnodet-bot
left a comment
There was a problem hiding this comment.
New modello-ant-task module — a well-structured Ant wrapper for Modello code generation with comprehensive tests covering nested elements, validation, and multiple generator goals. Good work overall, but a few issues worth addressing.
Issues found:
-
Model file existence not validated before generate() —
licenseFilegets anexists()check and a clear error, but model files in themodelslist do not. If a user passes a path that doesn't exist on disk, the error will come from deep inside Modello with a confusing stack trace. -
LicenseElement.addText()/setText()silently overwrite each other — Ant'saddTextconvention is for PCDATA content (Ant calls it with the text between the opening and closing tags). If a user sets both<license file="...">some text</license>, the PCDATA call toaddTextwill silently wipe the setter result or vice versa. Consider appending inaddText, or documenting that only one offile/ inline text is supported at a time. -
Platform-default charset in tests —
FileWriter(File)andnew String(byte[])both use the platform default charset (not UTF-8). Tests that write a model file withFileWriterand then verify the generated output withnew String(Files.readAllBytes(...))will silently misbehave on platforms where the default charset isn't UTF-8. Usenew FileWriter(file, StandardCharsets.UTF_8)andnew String(bytes, StandardCharsets.UTF_8)throughout. -
Missing Javadoc on public API —
ModelloTaskand its public methods are undocumented. Even a minimal class-level doc explaining what the task does and which attributes are required would go a long way.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
Platform default charset may differ. Files.newBufferedWriter instead of FileWriter(File, Charset): module targets Java 8.
Checked up front so no goal runs on a partial model list.
File plus text, or text attribute plus nested text, silently dropped one source. Nested text chunks are appended.
|
Thanks for the review. All four points are addressed, one commit each:
|
What does this change?
This adds the modello-ant-task I was developing standalone as module to upstream modello
It is a proposal ready for discussion. The syntax of the task is similar to what modello-maven-plugin uses. This tasks allows bootstrapping using generated ant builds of different packages that would otherwise need modello-maven-plugin.