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

Integrated yaml language server #6371

Merged
merged 5 commits into from
Oct 11, 2017
Merged

Integrated yaml language server #6371

merged 5 commits into from
Oct 11, 2017

Conversation

JPinkney
Copy link
Contributor

Signed-off-by: jpinkney josh.pinkney@mail.utoronto.ca

What does this PR do?

This PR integrates the yaml language server (https://github.com/redhat-developer/yaml-language-server) into eclipse che.

pr1
pr2
pr3

What issues does this PR fix or reference?

redhat-developer/rh-che#213

Changelog

Integrated yaml language server

Release Notes

Integrated yaml language server

Docs PR

@codenvy-ci
Copy link

Can one of the admins verify this patch?

1 similar comment
@codenvy-ci
Copy link

Can one of the admins verify this patch?

@riuvshin
Copy link
Contributor

ci-build

@codenvy-ci
Copy link

Build # 3744 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/3744/ to view the results.

riuvshin
riuvshin reviewed Sep 20, 2017
View reviewed changes
agents/ls-yaml/pom.xml Outdated
<parent>
<artifactId>che-agents-parent</artifactId>
<groupId>org.eclipse.che</groupId>
<version>5.18.0-SNAPSHOT</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

please update version to current 5.19.0-SNAPSHOT

agents/ls-yaml/src/main/resources/org.eclipse.che.ls.yaml.json
"description": "Yaml intellisense",
"dependencies": [],
"properties": {}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

missing new line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 9cf7ca0. For some reason github doesn't show the correction.

agents/ls-yaml/src/main/resources/org.eclipse.che.ls.yaml.script.sh Outdated

touch ${LS_LAUNCHER}
chmod +x ${LS_LAUNCHER}
echo "nodejs ${LS_DIR}/./out/server/src/server.js --stdio" > ${LS_LAUNCHER}
Copy link
Contributor

Choose a reason for hiding this comment

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

missing new line

Copy link
Contributor

Choose a reason for hiding this comment

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

./out -> out

agents/ls-yaml/src/main/resources/org.eclipse.che.ls.yaml.script.sh
@@ -0,0 +1,168 @@
#
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need shebang here? and also set -e ?

Copy link
Contributor

@tolusha tolusha Sep 21, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

No shebang

plugins/plugin-yaml/che-plugin-yaml-ide/pom.xml Outdated
<parent>
<artifactId>che-plugin-yaml-parent</artifactId>
<groupId>org.eclipse.che.plugin</groupId>
<version>5.18.0-SNAPSHOT</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

please update version to current 5.19.0-SNAPSHOT

plugins/plugin-yaml/che-plugin-yaml-server/pom.xml Outdated
<dependency>
<groupId>org.eclipse.che.core</groupId>
<artifactId>che-core-api-core</artifactId>
<version>5.18.0-SNAPSHOT</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

use ${project.version} here, and that is a wrong place for declare version

plugins/plugin-yaml/che-plugin-yaml-server/pom.xml Outdated
<dependency>
<groupId>javax.inject</groupId>
<artifactId>javax.inject</artifactId>
<version>1</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

you should not have dependency version declared here.

plugins/plugin-yaml/che-plugin-yaml-server/pom.xml Outdated
<dependency>
<groupId>javax.ws.rs</groupId>
<artifactId>javax.ws.rs-api</artifactId>
<version>2.0</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

you should not have dependency version declared here.

plugins/plugin-yaml/che-plugin-yaml-shared/pom.xml Outdated
<parent>
<artifactId>che-plugin-yaml-parent</artifactId>
<groupId>org.eclipse.che.plugin</groupId>
<version>5.18.0-SNAPSHOT</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

update version

plugins/plugin-yaml/pom.xml Outdated
<parent>
<artifactId>che-plugin-parent</artifactId>
<groupId>org.eclipse.che.plugin</groupId>
<version>5.18.0-SNAPSHOT</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

update version

tolusha
tolusha reviewed Sep 27, 2017
View reviewed changes
...src/main/java/org/eclipse/che/plugin/yaml/ide/preferences/YamlExtensionManagerPresenter.java Outdated
yamlPreferences.add(newYamlPref);
}
} catch (Exception e) {
System.out.println(e);
Copy link
Contributor

Choose a reason for hiding this comment

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

Use logger pls.

@codenvy-ci
Copy link

Build # 3851 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/3851/ to view the results.

@riuvshin
Copy link
Contributor

riuvshin commented Sep 27, 2017 •
edited
Loading

hey @JPinkney build of that PR failed on CI due to dependencies problem

18:08:37 [INFO] Add the following to your pom to correct the missing dependencies: 
18:08:37 [INFO] 
18:08:37 <dependency>
18:08:37   <groupId>org.slf4j</groupId>
18:08:37   <artifactId>slf4j-api</artifactId>
18:08:37   <version>1.7.24</version>
18:08:37 </dependency>

don't forget to sort pom after adding dep ;)

@codenvy-ci
Copy link

Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/3858/

@benoitf
Copy link
Contributor

benoitf commented Oct 10, 2017

@evidolob @vparfonov @skabashnyuk ?

@benoitf benoitf added kind/enhancement A feature request - must adhere to the feature request template. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. team/osio labels Oct 10, 2017
@vparfonov
Copy link
Contributor

Basically ok for me i just aware about location of binaries described here link. Is it legal @skabashnyuk ?

@l0rd l0rd merged commit 1870d7d into eclipse-che:master Oct 11, 2017
@l0rd l0rd added this to the 5.19.0 milestone Oct 11, 2017
@l0rd l0rd removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Oct 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@riuvshin riuvshin riuvshin left review comments

@benoitf benoitf benoitf approved these changes

@skabashnyuk skabashnyuk skabashnyuk approved these changes

@vparfonov vparfonov vparfonov approved these changes

@tolusha tolusha tolusha approved these changes

@evidolob evidolob Awaiting requested review from evidolob

Assignees
No one assigned
Labels
kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Milestone
5.19.0
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@JPinkney @codenvy-ci @riuvshin @benoitf @vparfonov @skabashnyuk @tolusha @l0rd