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

Added gitpod.Dockerfile #115

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

iqbalrohail
Copy link
Contributor

@iqbalrohail iqbalrohail commented Oct 28, 2022

Accommodates Java 17 requirement

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • As this plugin is tested with java 17 Test with Java 17 #98 , I have added a .gitpod.Dockerfile , which will allow developers to easily compile/test the plugin with JDK-17 while keeping the default JDK-11.

@iqbalrohail iqbalrohail requested a review from a team as a code owner October 28, 2022 14:19
@jmMeessen
Copy link
Contributor

jmMeessen commented Oct 28, 2022

Hello @iqbalrohail ,

Thank you for your pull request.
Could you explain/elaborate in the PR description what is the problem you are trying to solve with this PR? If I remember well the default Gitpod image (at least 2 months ago) defaulted to a Java 11.
Introducing a dockerfile adds a lot of delay in starting up the environment.

I am further puzzled because you are not including the dockerfile in the gitpod configuration.

@iqbalrohail
Copy link
Contributor Author

yes there should be configurations for this in gipod.yml as well

direct to dockerfile to use updated docker image
@iqbalrohail
Copy link
Contributor Author

it will use an updated Gitpod docker image (when using a non-default JDK or tools)

@jmMeessen
Copy link
Contributor

Can you please elaborate ? I don't understand what you are trying to achieve.

Especially as you are resetting the default JDK to JDK11 ?

My guess is that you want to give the opportunity to developers to easily compile/test the plugin with the bleeding-edge JDK17 (as it is now validated in the Jenkinsfile, see #98 ), while keeping the default JDK to 11. If this is the case, it would be worthwhile to add a note in a dedicated doc file (or start a CONTIBUTING.md doc) how to proceed. WDYT?

@iqbalrohail
Copy link
Contributor Author

As i checked that this can be compiled/test with jdk17, so I just added the configurations to check/test this with JDK 17 in a easy way but the default jdk is not changing , it will be jdk 11

@jmMeessen
Copy link
Contributor

So my guess was right. Very good initiative, @iqbalrohail.

As you have seen, the explanation of what you do and its intend is critical (if you want people to benefit from your work).

I believe that we need to clarify this at least in the PR description. How about adding a note about this in a special doc file, explaining how to switch from JDK when in Gitpod?

@iqbalrohail
Copy link
Contributor Author

I have clarified this in PR description

@jimklimov
Copy link
Contributor

And how is the eclipse style file change (formatter lines) related to this? :)

@iqbalrohail
Copy link
Contributor Author

@jimklimov sorry i didn't have any idea on this ..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants