eclipse-ee4j / starter

Eclipse Starter for Jakarta EE
Eclipse Public License 2.0
50 stars 38 forks source link

Indent velocity macros #327

Open OndroMih opened 1 month ago

OndroMih commented 1 month ago

With a maven plugin that removes the indentation in the artifact because velocity would leave it in the final files.

This makes it easier to read and write the source code and the indented space has no impact on generated files.

This change doesn't change any behavior of the final archetype artifact, only indentation in the source files.

Passes the nightly build executed against my branch: https://github.com/OndroMih/jakartaee-starter/actions/runs/10427857106

eclipse-starter-bot commented 1 month ago

Can one of the admins verify this patch?

m-reza-rahman commented 1 month ago

Please look into build failures. Also a good idea to run the nightly build to try to rule out unintended side effects.

m-reza-rahman commented 4 weeks ago

Did you check to see if the nightly build also works? Since this could break things, I’ll need to look carefully before I merge. I am off this week, so may be a little while. I do want to get the Docker support in sooner rather than later, so it’s a priority.

OndroMih commented 4 weeks ago

I've fixed the build failures. They happened because the Github action uses an old maven and thus old antrun plugin, which for reasons unknown to me didn't execute the target at all. The latest antrun plugin fixed that and the velocity files are properly un-indented.

I also started a nightly build in my branch, it's still running, without any failure so far: https://github.com/OndroMih/jakartaee-starter/actions/runs/10427857106

m-reza-rahman commented 4 weeks ago

Trying to understand - so the Archetype will now require a newer version of Maven? How new? Is this really worth it or will this cause users grief? Remember, we do still support quite old stuff in general in the Starter.

OndroMih commented 4 weeks ago

No, no need for newer maven. All works normally, without any change to the CI configuration.. Sorry for confusion. I just needed to define explicit version of antrun plugin to use, not rely on the default provided by maven, which was too old. When I set explicit antrun version to 3.1.0 all works now.

This pull request has no impact on users, the generated archetype will be the same as before. It only allows us to have indentation in the files with velocity markup, while the resulting files in the archetype are the same as before. The antrun plugin removes the indentation. In short, the source code is different, more readable, but the final archetype is the same as before.

m-reza-rahman commented 4 weeks ago

OK. I’ll take a look ASAP.