build.sh is a bad idea, just use maven

kpalang / mirth-sample-plugin

A sample NextGen Connect plugin.

Apache License 2.0

25 stars 8 forks source link

build.sh is a bad idea, just use maven #14

Closed jbeckers closed 2 weeks ago

jbeckers commented 1 month ago

I just tried to build https://github.com/jonbartels/bootermcgooter, and I naïvely updated mirth-plugin-maven-plugin.version from 2.0.1 to 3.0.0

mvn install created a plugin zip just like before, that MC even installed successfully.

Then after MC reboot, it failed with a class not found exception.

Turns out 3.0.0 builds a plugin.xml without <library path="bootermcgooter-server.jar" type="SERVER"/>, while 2.0.1 does include that line, because in 3.0.0 there's an extra step.

That's:

not great DX: needs two commands
not IDE-friendly: needs a build.sh instead of IDE maven integration
not very discoverable: I'd presumed the major version upgrade was due to the kotlin rewrite, not due to the extra step

I'm no maven expert, but I'd guess you can tell maven to run the extra step automatically?

jonbartels commented 4 weeks ago

Is this an issue with the sample plugin+guide or with my bootermcgooter plugin?

it failed with a class not found exception.

What exception did you get?

jbeckers commented 4 weeks ago

It's an issue with the maven plugin. In version 2 you could build a working plugin zip with just mvn package. In version 3 the same command builds a broken plugin zip, and an extra step is needed to fix the zip.

kpalang commented 2 weeks ago

build.sh is quite a good idea if the build process involves calling an external tool, I've even brought it out in the documentation with samples when to use one over the other.

Additionally, since we're working with a multi-module project, any steps specified in the parent pom.xml get run for each submodule. But we still need configuration to specify actions that should be performed on the whole project once. In v2 that place was the distribution submodule, in v2 that place was moved outside of maven to simplify the setup.

jbeckers commented 2 weeks ago

I don't agree that the (optional!) use of en external tool should impact everyone.

I don't agree that getting rid of "distribution" simplifies the setup. Quite the contrary.

Either way, it's bad DX to undermine IDE, CI, ... support.

kpalang commented 2 weeks ago

I don't agree that the (optional!) use of en external tool should impact everyone.

Then use the build steps directly? What's issue here? This is a sample implementation and you're complaining about a plugin someone else made? Additionally, you do realize this is unpaid work, built for my needs, and in my free time? You are free to fork the repository or submit pull requests if you find things you'd like changed.

I don't agree that getting rid of "distribution" simplifies the setup. Quite the contrary.

How does it complicate the setup? In my opinion moving the actions that using Maven would require long declaration files and hard-coding Maven lifecycle stages into external short commands with trivial ordering in a .sh file delivers drastic simplification.

Closing as unproductive.

jbeckers commented 2 weeks ago

The issue is simple: your current plugin requires using a .sh for something maven can do, whereas your previous plugin version didn't.

It's clear you think this is a good idea. Lets' agree to disagree.