maven-plugin-testing
maven-plugin-testing copied to clipboard
[MPLUGINTESTING-62] target maven 3.6.0 and Plexus 2.0.0
pretty frustrating that in 2019 people are still targeting ancient Java versions. In this pull request I've set JDK 8 as the target and also updated to a more recent maven api level and updated junit to the newer org.junit.vintage:junit-vintage-engine equivelant.
Any change of a new release? 3.3.0 was released years ago
Thank you for the PR, but you wasted your time. Most of the changes will be rejected. You can refactor your code to submit non-intrusive changes only.
All three aspects of the PR could be viewed that way. Is there a specific part that's concerning; JDK version, mvn version, or junit change?
I don't mind making changes. Just updating the targeted maven version would be good.
I do think that ASF shoud start targeting JDK 8 (even 7 would be an improvement over trying to support 6 through 13) but that should be in the parent pom, as per MPOM-222.
Our major goal is to release Maven 3.6.x. I do not know when Java 1.8 would be in Maven and I have not noticed any brainstorming, plan and email discussion with the whole team. I have noticed one Jira issue regarding Java 1.8 but I would say making such a change in the incremental version 3.6.2 is more unexpected step than in the version 3.7.0 or 4.0.0.
ok, I'm going to undo the JDK and junit change so that the PR is purely about changes needed to target the maven 3.6.0 API
edit: seems that's going to require JDK 7 as minimum, so will include that.
Looking at the code I see: generics (Java5), @Override on inherited interface methods (Java6). I don't see any changes that require a newer version of Java or Maven API. However, 7 is the new minimum, otherwise we can't test it with Java 12 and 13. That would also make it possible to apply: try-with-resources, diamond operators. As you might have noticed: upgrading is more than changing numbers in the pom.
Yes the update is just so that this plugin can work with newer code bases not because of requirements in the code. I'm currently working on a maven plugin and found this plugin troublesome because I use latest maven and Java.
There's some other changes I'd like to submit but wanted to see what happens with this first. For example, I'd like to change methods such as this:
protected Mojo lookupMojo( String goal, String pluginPom )
throws Exception
{
return lookupMojo( goal, new File( pluginPom ) );
}
to this:
protected <T extends Mojo> T lookupMojo( String goal, String pluginPom, Class<T> type)
throws Exception
{
return type.cast(lookupMojo(goal, new File(pluginPom)));
}
perhaps leaving the old version with @Deprecated
@SingingBush Let's separate the purpose of https://github.com/apache/maven-plugin-testing/pull/8#issuecomment-520244222 in a new PR.
@SingingBush
regarding this PR, there are rules to first issue a new Jira tickets, see https://issues.apache.org/jira/browse/MPLUGINTESTING
and then write name of PR in the form
[MPLUGINTESTING-12345] Jira title
and squash/revert commits into one commit with commit name again [MPLUGINTESTING-12345] Jira title.
done
protected <T extends Mojo> T lookupMojo( String goal, String pluginPom, Class<T> type)
This is just moving the cast to an argument. If you really want to change this, you should change it to
protected <T extends Mojo> T lookupMojo( Class<T> type, String pluginPom )
and extract the name (=goal) from its Mojo Annotation.
@rfscholte I've created https://issues.apache.org/jira/browse/MPLUGINTESTING-63 for that. My local changes were to simply move the cast into your code for now. I'll take a look at extracting it from the Mojo annotation, probably later this week.
the other changes I was looking at doing will be a seperate PR linked to MPLUGINTESTING-63.
Is anything else required for this PR? Can it be merged?
For this PR I don't see the need for changing the Maven version, it should be picked up by the direct as direct dependency of the plugin you're testing.
Bumping java.version + some other dependencies is good enough. Add that list to jira and adjust the title, because that's what we use for the release notes.
@Tibor17 @rfscholte I've rebased with master so that this PR is now simply changes to target Maven 3.6.0
Almost all our plugins are still compatible with Maven 3.0, pushing the Maven dependency to 3.6.0 will make it very hard to keep using this plugin. I noticed that it used to be 3.2.5, but I would prefer 3.1.1 when possible, otherwise keep it this value. I've noticed that especially in the early Maven 3 releases the testing harness written in a compatible way. In the end, the developer won't notice the difference, because the version of the maven dependencies of his plugin are leading.
So looks like we have the blocker to use this plugin where we agree on minimum to 3.1.0. One more challenge
Is this still in progress?