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.

Sign up for GitHub

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

Jump to bottom

Added support for java.jdt.ls.java.home vs-code configuration #1169

Merged
merged 4 commits into from
Feb 28, 2022
Merged

Added support for java.jdt.ls.java.home vs-code configuration #1169

merged 4 commits into from
Feb 28, 2022

Conversation

ravihara
Copy link
Contributor

No description provided.

@ghost
Copy link

ghost commented Jan 30, 2022 •
edited by ghost
Loading

CLA assistant check
All CLA requirements met.

@ravihara ravihara mentioned this pull request Jan 30, 2022
Open
CsCherrYY
CsCherrYY requested changes Feb 8, 2022
View reviewed changes
Copy link
Collaborator

@CsCherrYY CsCherrYY left a comment

Choose a reason for hiding this comment

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

LGTM overall, and some small suggestions about wording.

extension/src/util/config.ts Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@CsCherrYY
Copy link
Collaborator

@ravihara could you run ./gradlew format before committing since the CI reports some linting issues.

@ravihara
Copy link
Contributor Author

Hi @CsCherrYY I have done the required changes as mentioned in the above conversation. Please review the changes and take it forward. Thanks.

I have run ./gradlew format and ./gradlew build to check things.

@CsCherrYY
Copy link
Collaborator

@ravihara the changes LGTM. There are merge conflicts since we have updated all the deps in #1177. Could you just rebase and resolve all the conflicts? BTW could you exclude the changes in npm-package/package-lock.json(which is not used now), then I think we can merge this PR.

@CsCherrYY CsCherrYY changed the base branch from main to develop February 21, 2022 07:56
@CsCherrYY
Copy link
Collaborator

@ravihara please note that we have moved our default branch to develop and I have changed your pr's target branch to develop, thus, when rebasing, the target branch should be develop.

@ravihara
Copy link
Contributor Author

ravihara commented Feb 27, 2022 •
edited
Loading

@ravihara please note that we have moved our default branch to develop and I have changed your pr's target branch to develop, thus, when rebasing, the target branch should be develop.

Hi @CsCherrYY , sorry for the delay! Got tied up with the work. I have done the necessary changes. Please review it again and take it forward.

@CsCherrYY
Copy link
Collaborator

thank you @ravihara !

@CsCherrYY CsCherrYY merged commit f295c0e into microsoft:develop Feb 28, 2022
@CsCherrYY CsCherrYY added the enhancement New feature or request label Feb 28, 2022
@CsCherrYY CsCherrYY added this to the February 2022 milestone Feb 28, 2022
@CsCherrYY CsCherrYY mentioned this pull request Feb 28, 2022
Closed
3 tasks
CsCherrYY pushed a commit that referenced this pull request Feb 28, 2022
@ravihara @CsCherrYY
feat: Added support for java.jdt.ls.java.home vs-code configuration (#…
d74e9cf 
…1169)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CsCherrYY CsCherrYY CsCherrYY approved these changes

@Eskibear Eskibear Awaiting requested review from Eskibear

@jdneo jdneo Awaiting requested review from jdneo jdneo is a code owner

@testforstephen testforstephen Awaiting requested review from testforstephen testforstephen is a code owner

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
February 2022
Development

Successfully merging this pull request may close these issues.
2 participants
@ravihara @CsCherrYY