Skip to content

Add ant task to modello - #606

Open
fridrich wants to merge 7 commits into
codehaus-plexus:masterfrom
fridrich:ant-task
Open

fridrich wants to merge 7 commits into
codehaus-plexus:masterfrom
fridrich:ant-task

Conversation

@fridrich

Copy link
Copy Markdown

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.

@fridrich

Copy link
Copy Markdown
Author

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.

@fridrich
fridrich force-pushed the ant-task branch 2 times, most recently from 40dfef5 to 1d36e0b Compare September 21, 2026 07:52
@slachiewicz slachiewicz added the enhancement New feature or request label Sep 21, 2026
@fridrich
fridrich force-pushed the ant-task branch 2 times, most recently from ce34e2d to 3a59e90 Compare September 26, 2026 07:57
@fridrich

Copy link
Copy Markdown
Author

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.

@fridrich

Copy link
Copy Markdown
Author

Even in this one, I went through the mojo's behaviour once again and tried to align as much as I could.

@fridrich

Copy link
Copy Markdown
Author

When the CLI changes enter, I will quickly rebase it to use the ModelloParameterConstants instead of raw strings

@gnodet-bot gnodet-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Model file existence not validated before generate() — licenseFile gets an exists() check and a clear error, but model files in the models list 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.

  2. LicenseElement.addText() / setText() silently overwrite each other — Ant's addText convention 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 to addText will silently wipe the setter result or vice versa. Consider appending in addText, or documenting that only one of file / inline text is supported at a time.

  3. Platform-default charset in tests — FileWriter(File) and new String(byte[]) both use the platform default charset (not UTF-8). Tests that write a model file with FileWriter and then verify the generated output with new String(Files.readAllBytes(...)) will silently misbehave on platforms where the default charset isn't UTF-8. Use new FileWriter(file, StandardCharsets.UTF_8) and new String(bytes, StandardCharsets.UTF_8) throughout.

  4. Missing Javadoc on public API — ModelloTask and 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.
@fridrich

fridrich commented Nov 5, 2026 •

Copy link
Copy Markdown
Author

Thanks for the review. All four points are addressed, one commit each:

  1. Model file existence: every <model> is checked before any goal runs; a missing file fails with Model file not found: <absolute path>.
  2. License text/file ambiguity: ambiguous configuration now fails instead of silently dropping one source, both for <license file="...">text</license> / text attribute plus nested text, and for the task's licenseFile plus licenseText attributes. Nested text chunks are appended, following Ant's convention. The README documents that only one source is allowed.
  3. Platform-default charset in tests: tests write and read with explicit UTF-8. I used Files.newBufferedWriter(path, StandardCharsets.UTF_8) rather than FileWriter(File, Charset) or Files.readString, because the module targets Java 8.
  4. Javadoc: added class-level documentation (purpose, required attributes and nested elements) and a one-line doc for every public setter, nested element handler and nested element class.

@fridrich
fridrich requested a review from gnodet-bot November 5, 2026 15:53
@fridrich fridrich closed this Nov 5, 2026
@fridrich fridrich reopened this Nov 5, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants