Skip to content
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.

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

Upgrades to Junit 4.12 #96

Closed
wants to merge 3 commits into from
Closed

Conversation

hansjoachim
Copy link

Upgrade to latest version of Junit, 4.12. This is now used for all modules, including integration which seemed stuck on a different, older version.

mvn clean install ran successfully.

@mattbishop
Copy link
Contributor

Pretty straightforward, any complaints?

@@ -29,7 +29,7 @@
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>3.8.1</version>
<version>${junit.version}</version>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I just noticed that in addition to here in dependencies, junit is also defined as a dependency of maven-antrun-plugin below. Should that be updated as well perhaps?

@hansjoachim
Copy link
Author

Pretty straightforward, any complaints?

Not sure if you're asking me or other reviewers, but I don't have any particular complaints.

There is an open question on what to do with the integration-module though. As mentioned above, it seems to use an older version than the others and also specify this as a dependency for the maven-antrun-plugin. I'm not sure why this is the case, so I'm open to suggestions on how to proceed with this.

@mattbishop
Copy link
Contributor

I would say upgrade for antrun-plugin and see if the tests all pass. Or maybe upgrade ant run-plugin if it's out of date and see if it brings in a newer version of junit. JUnit has done a pretty good job with backwards compatibility over the years.

@hansjoachim
Copy link
Author

Or maybe upgrade ant run-plugin if it's out of date and see if it brings in a newer version of junit.

It didn't seem to do so without specifying it manually, so I set it to the same junit version as used elsewhere.

I thought about upgrading the plugin to a newer release as well, which can probably be done for several others too. If you look at the output from mvn versions:display-plugin-updates, some plugins require a minimum version of maven. (I assume this is because they use features or APIs which doesn't exist in older versions of maven) I don't know slf4j's policy on prerequisites or which maven versions are targeted, though if you check the output it looks some plugins (like the version of maven-site-plugin) already has some implicit requirements so effectively there should be little change. I can look into the plugin upgrades if that sounds ok.

@hazendaz
Copy link
Contributor

Antrun latest only requires maven 2.2.1 to run. I'd suggest if antrun is really needed still to get it upgraded all the way to latest and update ant as well to latest. Probably a separate pull request for that.

As for updating others, this project is using maven-site-plugin 3.3 which is on maven 3 core. I'm sure there are a few others, haven't looked. So that pretty much tells me everything can be updated without harm. Before going all crazy though, I'd love someone to take a look at my pull request to cleanup the builds using maven best practices. #88

Then probably another pull request down the road to update all the maven plugins. Once everything is at least on par with a maven 3 core build, then the builds can be done in parallel and thus much faster. Food for thought anyway!

@hansjoachim
Copy link
Author

Probably a separate pull request for that.

Yes, I would have done that as part of another pull request. This was mainly because I saw junit was outdated.

Before going all crazy though, I'd love someone to take a look at my pull request to cleanup the builds using maven best practices. #88

Thanks for the reminder, I actually saw that earlier. I agree that should probably land before the plugin upgrades to avoid merge conflicts etc...

@hansjoachim
Copy link
Author

Hi again. What is the status of this request? Is there anything holding it up which I can look into? :)

@mleegwt
Copy link

mleegwt commented Feb 28, 2019

This pull request should be closed. The changes have been applied elsewhere I think.

@hansjoachim
Copy link
Author

Yes, looks like the changes have made their way into master :)

@hansjoachim hansjoachim closed this May 5, 2019
@hansjoachim hansjoachim deleted the junit4.12 branch May 5, 2019 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants