Skip to content

Migrate imagej-maven-plugin #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
ctrueden merged 16 commits into from
Sep 5, 2018
Merged

Migrate imagej-maven-plugin #15

ctrueden merged 16 commits into from
Sep 5, 2018

Conversation

@stelfrich
Copy link
Member

@stelfrich stelfrich commented Aug 10, 2018 •
edited
Loading

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:

  • Fix license headers of migrated files
  • Rename imagej* to scijava* in AbstractCopyJarsMojo

@stelfrich
Add imagej-maven-plugin's sources
904c25a 
The sources are based on imagej/imagej-maven-plugin#9240da2 with the
sources moved into the correct packages already. In addition to the
already available imagej.* properties, scijava.* properties have been
added with compatibility between those properties.
@stelfrich
Add more detailed Javadocs
e89cc12
@stelfrich
Add imagej-maven-plugin's integration tests
54d3593
@stelfrich
Update property names in integration tests
667ce7b
@stelfrich
Add imagej-maven-plugin's lifecycle mapping
08e59f0
@stelfrich stelfrich force-pushed the migrate-imagej-maven-plugin branch from 6cb17f0 to c7889ef Compare August 10, 2018 05:15
@stelfrich
Merge POMs
5b3d205 
Also bumps the parent POM to the more recent version of pom-scijava
of imagej-maven-plugin. Some dependencies have to be bumped to newer
versions as well, which also leads to some minor changes wrt. enforcer
rules.
@stelfrich
Replace Javadoc descriptor with annotation
80176ca 
This changes is required due the bump of pom-scijava in the previous
commit.
@stelfrich
Merge READMEs
3e7c27c
@stelfrich
Add compatibility integration test
744d8f2
@stelfrich
Add compatibility for legacy configuration params
6b5cd6b
@stelfrich stelfrich force-pushed the migrate-imagej-maven-plugin branch from c7889ef to 6b5cd6b Compare August 10, 2018 05:20
@stelfrich stelfrich changed the title Migrate imagej maven plugin Migrate imagej-maven-plugin Aug 10, 2018
@stelfrich stelfrich mentioned this pull request Aug 10, 2018
Closed
@ctrueden
Copy link
Member

ctrueden commented Aug 10, 2018

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
Copy link
Member

imagejan commented Aug 11, 2018

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
Copy link
Member Author

stelfrich commented Aug 13, 2018

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
Copy link
Member Author

stelfrich commented Aug 13, 2018

@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?

This was referenced Aug 13, 2018
Closed
Closed
@stelfrich stelfrich changed the title Migrate imagej-maven-plugin [WIP] Migrate imagej-maven-plugin Aug 30, 2018
@stelfrich stelfrich changed the title [WIP] Migrate imagej-maven-plugin Migrate imagej-maven-plugin Aug 31, 2018
@ctrueden ctrueden force-pushed the migrate-imagej-maven-plugin branch from 0b40dd2 to ba75487 Compare September 5, 2018 18:43
@ctrueden ctrueden merged commit e694f81 into master Sep 5, 2018
@ctrueden ctrueden deleted the migrate-imagej-maven-plugin branch September 5, 2018 18:43
ctrueden pushed a commit that referenced this pull request Nov 13, 2021
@davidburkhart
Merge pull request #15 from SimY4/master
93b0cd8 
Allow to specify packages to include/exclude.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@stelfrich @ctrueden @imagejan