search

scijava / scijava-maven-plugin

A Maven plugin to manage development of SciJava-based software.
BSD 2-Clause "Simplified" License
3 stars 5 forks source link

Migrate imagej-maven-plugin #15

Closed stelfrich closed 6 years ago

stelfrich commented 6 years ago

This PR migrates the functionality of imagej-maven-plugin as of 9240da2 to scijava-maven-plugin.

It keeps backwards compatibility as far as it's possible. This means, supporting the legacy properties imagej.app.directory, imagej.app.subdirectory, imagej.deleteOtherVersions, and delete.other.versions. This means that projects that have used imagej-maven-plugin implicitly by making pom-scijava the parent for their project, should see no difference once, we switch to scijava-maven-plugin in pom-scijava.

There is one point that I would like some input on: should the scijava.* properties take precedence over imagej.* ones if both are set simultaneously? Do you have an opinion, @ctrueden?

TODOs:

ctrueden commented 6 years ago

should the scijava.* properties take precedence over imagej.* ones if both are set simultaneously?

That seems the most intuitive to me, yeah. I would suggest not formally specifying behavior in that circumstance, though. Then if some unintuitive circumstance arises where it would be convenient for the precedence to go the other way, we can flip it. But I doubt it will be important.

imagejan commented 6 years ago

should the scijava.* properties take precedence over imagej.* ones if both are set simultaneously?

How about ignoring all imagej properties if at least one scijava property is set, and issuing a warning to make it transparent to the caller what's happening?

stelfrich commented 6 years ago

How about ignoring all imagej properties if at least one scijava property is set, and issuing a warning to make it transparent to the caller what's happening?

I like that and will start implementing it!

stelfrich commented 6 years ago

@ctrueden I have implemented @imagejan's suggestion and have only "defined" the behavior in a log message. Should we get rid of that as well?