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

Introduce ESLint extension #687

Merged
merged 6 commits into from
Nov 18, 2020
Merged

Introduce ESLint extension #687

merged 6 commits into from
Nov 18, 2020

Conversation

vitaliy-guliy
Copy link
Contributor

@vitaliy-guliy vitaliy-guliy commented Nov 11, 2020

Signed-off-by: Vitaliy Gulyy vgulyy@redhat.com

What does this PR do?

Adds VS Code ESLint 2.1.1 extension to che-incubator/typescript plugin

Screenshot/screencast of this PR

What issues does this PR fix or reference?

eclipse-che/che#14070

How to test this PR?

Devfile to test

---
apiVersion: 1.0.0
metadata:
  name: eslint-2.1.1
projects:
  -
    name: che-theia
    source:
      type: git
      location: "https://github.com/eclipse/che-theia.git"
components:
  # Che-Theia without built-in ES Lint from https://github.com/eclipse/che-theia/pull/923
  - type: cheEditor
    alias: che-theia
    reference: https://raw.githubusercontent.com/chepullreq4/pr-check-files/master/che-theia/pr-923/che_theia_meta.yaml
    memoryLimit: 768Mi
  -
    type: chePlugin
    id: redhat/vscode-yaml/latest
  - 
    type: chePlugin
    id: ms-vscode/node-debug2/latest
    preferences:
      debug.node.useV3: false
  -
    type: chePlugin
    reference: https://raw.githubusercontent.com/eclipse/che-plugin-registry/eslint/v3/plugins/dbaeumer/vscode-eslint/2.1.1/meta.yaml
  # -
  #   type: chePlugin
  #   id: che-incubator/typescript/latest
  -
    alias: che-dev
    type: dockerimage
    image: quay.io/eclipse/che-theia-dev:next
    memoryLimit: "3Gi"
    mountSources: true
    endpoints:
      - name: 'theia-dev-flow'
      # - name: 'che-theia-link'
        port: 3010
        attributes:
          type: ide-dev
          protocol: http
          public: 'true'
          discoverable: 'false'
commands:
  -
    name: '1.1 che-theia :: build'
    actions:
      - type: exec
        component: che-dev
        workdir: /projects/che-theia
        command: |
          yarn
          echo -e "\e[32mDone.\e[0m"

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Nov 11, 2020

@vitaliy-guliy Happy Path PR check [build 403] failed.

Re-trigger by [ci-test] or [ci-test-happy-path].

Link URL
console https://ci.centos.org/job/devtools-che-plugin-registry-pr-check-happy-path/403/console
artifacts http://artifacts.ci.centos.org/devtools/che/che-plugin-registry-prcheck/403/

Depending on failure reason, the artifacts or deployment may not be present.

@benoitf
Copy link
Contributor

benoitf commented Nov 11, 2020

Hi, why eslint is in typescript plug-in ?

@vitaliy-guliy
Copy link
Contributor Author

Hi, why eslint is in typescript plug-in ?

I just thought that's the best place for it. Do you have other ideas?
Whether it better to have an extra plugin to run eslint in the same container as Che-Theia?

@vitaliy-guliy
Copy link
Contributor Author

As it appeared, VS Code ES Lint is already introduced to Che-Theia built-ins by eclipse-che/che-theia#902
And moreover, it works fine.

It could be tested with this devfile

---
apiVersion: 1.0.0
metadata:
  name: eslint-che-theia
projects:
  -
    name: che-theia
    source:
      type: git
      location: "https://github.com/eclipse/che-theia.git"
components:
  - type: cheEditor
    alias: che-theia
    reference: https://raw.githubusercontent.com/chepullreq4/pr-check-files/master/che-theia/pr-902/che_theia_meta.yaml
    memoryLimit: 768Mi
  - type: chePlugin
    id: redhat/vscode-yaml/latest
  - 
    type: chePlugin
    id: ms-vscode/node-debug2/latest
    preferences:
      debug.node.useV3: false
  -
    alias: che-dev
    type: dockerimage
    image: quay.io/eclipse/che-theia-dev:next
    memoryLimit: "3Gi"
    mountSources: true
    endpoints:
      - name: 'theia-dev-flow'
      # - name: 'che-theia-link'
        port: 3010
        attributes:
          type: ide-dev
          protocol: http
          public: 'true'
          discoverable: 'false'
commands:
    name: '1.1 che-theia :: build'
    actions:
      - type: exec
        component: che-dev
        workdir: /projects/theia
        command: |
          yarn
          echo -e "\e[32mDone.\e[0m"

The editor was taken from the Devfile for Happy Path tests for PR 902

To test:

  • create a workspace
  • build Che-Theia or just install node dependencies
  • open any typescript file and check ES Lint

This is what I got for variable declaration
Screenshot from 2020-11-12 17-55-32

When there is no new-line character at the end of the file
Screenshot from 2020-11-12 18-00-37

For a line, having trailing spaces
Screenshot from 2020-11-12 18-00-46

@benoitf
Copy link
Contributor

benoitf commented Nov 12, 2020

I think it's a mistake that it went into built-in plug-ins
it's not a built-in

@benoitf benoitf reopened this Nov 12, 2020
@che-bot
Copy link
Contributor

che-bot commented Nov 12, 2020

@vitaliy-guliy Happy Path PR check [build 409] failed.

Re-trigger by [ci-test] or [ci-test-happy-path].

Link URL
console https://ci.centos.org/job/devtools-che-plugin-registry-pr-check-happy-path/409/console
artifacts http://artifacts.ci.centos.org/devtools/che/che-plugin-registry-prcheck/409/

Depending on failure reason, the artifacts or deployment may not be present.

@vitaliy-guliy
Copy link
Contributor Author

I think it's a mistake that it went into built-in plug-ins
it's not a built-in

NP, will be extracted into separate Che-Plugin soon.

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Nov 13, 2020

@vitaliy-guliy Happy Path PR check [build 411] failed.

Re-trigger by [ci-test] or [ci-test-happy-path].

Link URL
console https://ci.centos.org/job/devtools-che-plugin-registry-pr-check-happy-path/411/console
artifacts http://artifacts.ci.centos.org/devtools/che/che-plugin-registry-prcheck/411/

Depending on failure reason, the artifacts or deployment may not be present.

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>
@vitaliy-guliy
Copy link
Contributor Author

Three versions of ES Lint were added.
One latest stable version will be left after testing.

@che-bot
Copy link
Contributor

che-bot commented Nov 13, 2020

@vitaliy-guliy Happy Path PR check [build 412] failed.

Re-trigger by [ci-test] or [ci-test-happy-path].

Link URL
console https://ci.centos.org/job/devtools-che-plugin-registry-pr-check-happy-path/412/console
artifacts http://artifacts.ci.centos.org/devtools/che/che-plugin-registry-prcheck/412/

Depending on failure reason, the artifacts or deployment may not be present.

@vitaliy-guliy
Copy link
Contributor Author

Only 2.1.1 version is stable and works without errors.
2.1.3 and 2.1.8 seems to be working, but have some issues with displaying the editor tooltip when hovering a mouse over a problem
Screenshot from 2020-11-18 15-22-25

Let's focus on 2.1.1, it's also included to upstream Theia built-ins https://github.com/eclipse-theia/theia/blob/e2098321a66e96e72248b555ca32063127857a98/package.json#L153

We need to investigate the reason of the console errors for 2.1.3 and 2.1.8. And then we can always create another PR with more fresh ES Lint.

@vitaliy-guliy
Copy link
Contributor Author

vitaliy-guliy commented Nov 18, 2020

Devfile to test 2.1.1

---
apiVersion: 1.0.0
metadata:
  name: eslint-2.1.1
projects:
  -
    name: che-theia
    source:
      type: git
      location: "https://github.com/eclipse/che-theia.git"
components:
  # Che-Theia without built-in ES Lint from https://github.com/eclipse/che-theia/pull/923
  - type: cheEditor
    alias: che-theia
    reference: https://raw.githubusercontent.com/chepullreq4/pr-check-files/master/che-theia/pr-923/che_theia_meta.yaml
    memoryLimit: 768Mi
  -
    type: chePlugin
    id: redhat/vscode-yaml/latest
  - 
    type: chePlugin
    id: ms-vscode/node-debug2/latest
    preferences:
      debug.node.useV3: false
  -
    type: chePlugin
    reference: https://raw.githubusercontent.com/eclipse/che-plugin-registry/eslint/v3/plugins/dbaeumer/vscode-eslint/2.1.1/meta.yaml
  # -
  #   type: chePlugin
  #   id: che-incubator/typescript/latest
  -
    alias: che-dev
    type: dockerimage
    image: quay.io/eclipse/che-theia-dev:next
    memoryLimit: "3Gi"
    mountSources: true
    endpoints:
      - name: 'theia-dev-flow'
      # - name: 'che-theia-link'
        port: 3010
        attributes:
          type: ide-dev
          protocol: http
          public: 'true'
          discoverable: 'false'
commands:
  -
    name: '1.1 che-theia :: build'
    actions:
      - type: exec
        component: che-dev
        workdir: /projects/che-theia
        command: |
          yarn
          echo -e "\e[32mDone.\e[0m"

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>
@vitaliy-guliy vitaliy-guliy marked this pull request as ready for review November 18, 2020 14:10
@che-bot
Copy link
Contributor

che-bot commented Nov 18, 2020

@vitaliy-guliy Happy Path PR check [build 416] failed.

Re-trigger by [ci-test] or [ci-test-happy-path].

Link URL
console https://ci.centos.org/job/devtools-che-plugin-registry-pr-check-happy-path/416/console
artifacts http://artifacts.ci.centos.org/devtools/che/che-plugin-registry-prcheck/416/

Depending on failure reason, the artifacts or deployment may not be present.

v3/plugins/dbaeumer/vscode-eslint/2.1.1/meta.yaml Outdated Show resolved Hide resolved
vscode-extensions.json Outdated Show resolved Hide resolved
Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>
Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Nov 18, 2020

@vitaliy-guliy Happy Path PR check [build 417] failed.

Re-trigger by [ci-test] or [ci-test-happy-path].

Link URL
console https://ci.centos.org/job/devtools-che-plugin-registry-pr-check-happy-path/417/console
artifacts http://artifacts.ci.centos.org/devtools/che/che-plugin-registry-prcheck/417/

Depending on failure reason, the artifacts or deployment may not be present.

@che-bot
Copy link
Contributor

che-bot commented Nov 18, 2020

@vitaliy-guliy Happy Path PR check [build 418] failed.

Re-trigger by [ci-test] or [ci-test-happy-path].

Link URL
console https://ci.centos.org/job/devtools-che-plugin-registry-pr-check-happy-path/418/console
artifacts http://artifacts.ci.centos.org/devtools/che/che-plugin-registry-prcheck/418/

Depending on failure reason, the artifacts or deployment may not be present.

@ericwill ericwill merged commit 7c86713 into master Nov 18, 2020
@ericwill ericwill deleted the eslint branch November 18, 2020 20:57
@che-bot
Copy link
Contributor

che-bot commented Nov 19, 2020

@vitaliy-guliy Happy Path PR check [build 421] failed.

Re-trigger by [ci-test] or [ci-test-happy-path].

Link URL
console https://ci.centos.org/job/devtools-che-plugin-registry-pr-check-happy-path/421/console
artifacts http://artifacts.ci.centos.org/devtools/che/che-plugin-registry-prcheck/421/

Depending on failure reason, the artifacts or deployment may not be present.

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