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

chore: Add Dockerfiles for maven 3.6.1 #710

Closed
wants to merge 2 commits into from

Conversation

123Haynes
Copy link
Contributor

@123Haynes 123Haynes commented Aug 5, 2019

  • Ready for review
  • Follows CONTRIBUTING rules
  • Reviewed by Snyk internal team

What does this PR do?

This PR adds 2 new Dockerfiles.
One for for maven 3.6.1 with Java 8 and one for maven 3.6.1 with java 11

Currently there is only a dockerfile for maven 3.5.4 with java 8.
Given that there are several dockerfile for different gradle versions, I added a new Dockerfile for the latest maven version instead of updating the existing Dockerfile.

Where should the reviewer start?

How should this be manually tested?

Build the Dockerfiles 😃 and use the image on a maven project.

Any background context you want to provide?

What are the relevant tickets?

Maven 3.6.1 is the latest version of maven and Java 8 and 11 are the two most used java versions.

Screenshots

Additional questions

@123Haynes 123Haynes requested a review from a team as a code owner August 5, 2019 13:25
@ghost ghost requested review from lwywoo and miiila August 5, 2019 13:25
@CLAassistant
Copy link

CLAassistant commented Aug 5, 2019

CLA assistant check
All committers have signed the CLA.

@123Haynes 123Haynes changed the title Add a Dockerfile for maven 3.6.1 chore: Add Dockerfiles for maven 3.6.1 Aug 5, 2019
@123Haynes
Copy link
Contributor Author

Note that travis CI is failing because my fork does not have the necessary environment variables available.
This is completely unrelated to the 2 new Dockerfiles.

@lili2311
Copy link
Contributor

lili2311 commented Aug 5, 2019

hi @123Haynes! Thank you for this. Before we can officially update & merge we will need to update our test matrix and verify java 10 & 11 is ok to verify everything is working with the plugin. Let us update that and we can then merge this awesome PR as well :)

@123Haynes
Copy link
Contributor Author

@lili2311 Thanks for your reply 😄
As a side note: I don't think it's necessary to test against java 9 and 10 anymore because they were non lts versions and reached EOL quite a while ago.
Java 9 EOL: March 2018
Java 10 EOL: September 2018
Also these versions were not used by many people.
If you want to support non lts versions it would make more sense to always test against the latest, officially release version of java, which is 13 at the moment.

@miiila
Copy link
Contributor

miiila commented Aug 6, 2019

Hi @123Haynes, we've recently started testing our flows on Java 11 and everything looks smooth, so we expect to review and proceed with this PR soon. Stay tuned :-) And thank you for your patience.

@miiila
Copy link
Contributor

miiila commented Aug 7, 2019

Hi @123Haynes in order to run all the tests, I merged your PR to side branch and opened new one (#718). Thank you again for this contribution, hope to see it in master soon :-)

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.

4 participants