-
Notifications
You must be signed in to change notification settings - Fork 152
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
base: master
Are you sure you want to change the base?
Conversation
accomodates java 17 requirements
Hello @iqbalrohail , Thank you for your pull request. I am further puzzled because you are not including the dockerfile in the gitpod configuration. |
yes there should be configurations for this in gipod.yml as well |
direct to dockerfile to use updated docker image
it will use an updated Gitpod docker image (when using a non-default JDK or tools) |
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? |
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 |
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? |
I have clarified this in PR description |
And how is the eclipse style file change (formatter lines) related to this? :) |
@jimklimov sorry i didn't have any idea on this .. |
Accommodates Java 17 requirement