search

imagej / imagej-maven-plugin

[OBSOLETE] All goals migrated to scijava/scijava-maven-plugin
Other
4 stars 6 forks source link

Add -Ddelete.older.versions flag #15

Closed ctrueden closed 6 years ago

ctrueden commented 7 years ago

Probably named -Dimagej.deleteOlderVersions actually, but we need functionality like delete.other.versions but only when the existing libraries are older according to SemVer.

stelfrich commented 6 years ago

I would find it more intuitive to have a keep.newer.versions property that is true per default. I'd get confused if delete.other.versions=false and delete.older.versions=true: will it delete anything?

imagejan commented 6 years ago

How about having -Dimagej.deleteOtherVersions=always, -Dimagej.deleteOtherVersions=ifOlder and -Dimagej.deleteOtherVersions=never to avoid confusion with two parameters that have overlapping functionality?

stelfrich commented 6 years ago

@imagejan: I like it! The keep-newer-versions branch features a first version that deprecates the old delete.other.versions and maps delete.other.versions=true to imagej.deleteOtherVersions=always and delete.other.versions=false to imagej.deleteOtherVersions=never.

stelfrich commented 6 years ago

@ctrueden: The keep-newer-versions branch introduces a dependency on a Java implementation of SemVer (JSemVer). I am hesitant to use it since it doesn't seem to be maintained actively. While I could re-implement the parts of the SemVer specification that are relevant for our use-case at the moment (basically version1.greaterThan(version2)). We might want to check for compatibility in future based on SemVer. Any thoughts?

ctrueden commented 6 years ago

If JSemVer does what you need, why not just use it? Are you concerned we might find a bug and be unable to get it fixed upstream? The license is MIT so we can just fork it to the org.scijava groupId if needed.

stelfrich commented 6 years ago

Are you concerned we might find a bug and be unable to get it fixed upstream?

That was my major concern, although I was also hesitant about introducing another dependency and wanted to check with you. But you are right that we could fork it any time.

Also @ctrueden, are you fine with @imagejan's suggestion of -Dimagej.deleteOtherVersions=always / ifOlder / never? I have taken care to keep backwards compatibility, when delete.other.versions is set explicitly. If not, default behavior (delete.other.versions unset) is the same.

ctrueden commented 6 years ago

are you fine with @imagejan's suggestion of -Dimagej.deleteOtherVersions=always / ifOlder / never?

Yes, I think this is a great solution. (I'd suggest older rather than ifOlder, but that's a nitpick.)

Thanks for your efforts!