Skip to content

Test Maven plugin against latest Maven versions #26200

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

Closed
wants to merge 1 commit into from
Closed

Test Maven plugin against latest Maven versions #26200

wants to merge 1 commit into from

Conversation

slawekjaranowski
Copy link
Contributor

Maven version 3.8.1 is only bug fixes for 3.6.3 so can be used in it tests instead of 3.6.2

https://maven.apache.org/docs/3.8.1/release-notes.html
https://maven.apache.org/docs/3.6.3/release-notes.html

@pivotal-issuemaster
Copy link

@slawekjaranowski Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

@slawekjaranowski Thank you for signing the Contributor License Agreement!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 21, 2021
@snicoll
Copy link
Member

snicoll commented Apr 22, 2021

Thanks for the PR @slawekjaranowski.

Testing on 3.8.1 is a good idea but I wouldn't remove testing from the 3.6.x line. I wonder if rather we shouldn't bump the system requirements. We're currently stating 3.3+. Flagging for team attention to see what the rest of the team thinks.

@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Apr 22, 2021
@wilkinsona
Copy link
Member

wilkinsona commented Apr 22, 2021

I agree that we shouldn't drop 3.6.x from the versions that we test against. Instead, I think we should add 3.8.x alongside the existing versions for now and also bump the system requirements as a separate issue.

@slawekjaranowski
Copy link
Contributor Author

please consider that it tests on one maven versions take about 6 minutes (2,6 GHz 6-Core Intel Core i7)

@wilkinsona
Copy link
Member

wilkinsona commented Apr 22, 2021

The length of time that's taken to run the tests isn't ideal, but I don't think that's a good reason not to run them. We're in a similar situation with the Gradle plugin's tests where we've considered refining things a little to limit the number of tests that run against multiple versions. We could consider something similar with the Maven plugin's tests if it becomes problematic.

@slawekjaranowski
Copy link
Contributor Author

documentation fix: #26209

@snicoll
Copy link
Member

snicoll commented Apr 22, 2021

@slawekjaranowski can you please update your PR with the review above?

@slawekjaranowski
Copy link
Contributor Author

fixed.

by the way 3.8.1 could be as next build for 3.6.x - it contains only fix for CVEs security, more explanation at: https://maven.apache.org/docs/3.8.1/release-notes.html#why-does-this-version-have-the-value-3-8-1

@slawekjaranowski slawekjaranowski changed the title bump maven from 3.6.2 to 3.8.1 for integration tests bump maven version 3.6.2 to 3.6.3 and add 3.8.1 for integration tests Apr 22, 2021
@snicoll
Copy link
Member

snicoll commented Apr 22, 2021

We are aware of what 3.8.1 contains and it is not the same thing as 3.6.x.

@snicoll snicoll added type: task A general task and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Apr 22, 2021
@snicoll snicoll added this to the 2.5.0 milestone Apr 22, 2021
@snicoll snicoll self-assigned this Apr 22, 2021
@snicoll snicoll changed the title bump maven version 3.6.2 to 3.6.3 and add 3.8.1 for integration tests Test Maven plugin against latest Maven versions Apr 22, 2021
snicoll pushed a commit that referenced this pull request Apr 22, 2021
@snicoll snicoll closed this in a654380 Apr 22, 2021
@slawekjaranowski slawekjaranowski deleted the mvn-3.8-it-test branch April 22, 2021 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants