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

ESLint dependency #4

Closed
ogandose opened this issue Sep 17, 2019 · 8 comments · Fixed by #5
Closed

ESLint dependency #4

ogandose opened this issue Sep 17, 2019 · 8 comments · Fixed by #5
Labels
enhancement New feature or request released

Comments

@ogandose
Copy link
Contributor

Hi 🙂 I was just wondering one more thing, is it necessary to have this hard dependency on ESLint 5?

@nomego
Copy link
Contributor

nomego commented Sep 18, 2019

Probably not! We can likely switch it to >= 5 but that's what we're on and testing it with at the moment. Are you running this with ESLint 6?

@nomego nomego added the enhancement New feature or request label Sep 18, 2019
@nomego nomego added this to To do in October 1 - London Beer Flood via automation Sep 18, 2019
@ogandose
Copy link
Contributor Author

We're actually also using ESLint 5. After I posted this I noticed that I'm not really using your dependency since I'm installing them all separately to avoid running npm install since we have quite a lot of dependencies. Do you possibly have an idea how this could be avoided?

@ogandose
Copy link
Contributor Author

ogandose commented Sep 18, 2019

on:
  pull_request:
  push:
    branches:
      - master
 
jobs:
  checks:
    runs-on: ubuntu-latest
    steps:
    - name: Checkout
      uses: actions/checkout@master
      
    - name: install eslint
      run: | 
             npm install eslint@5.2.0
             npm install babel-eslint@^9.0.0
             npm install eslint-plugin-amd-imports@^4.0.0
             npm install eslint-plugin-jest@^21.18.0
             npm install eslint-plugin-prettier@^2.6.2
             npm install eslint-plugin-react@^7.10.0
             npm install eslint-plugin-react-hooks@^1.6.1
             npm install eslint-plugin-requirejs@^4.0.0
             npm install prettier@^1.18.2
             npm install @neovici/github-actions-eslint-annotator
         
    - name: ESLint Annotations
      run: npx github-actions-eslint-annotator
      env:
        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
        PARTIAL_LINTING: true

@nomego
Copy link
Contributor

nomego commented Sep 23, 2019

Is this the whole workflow? Why don't you want to do npm install?
The ESLint step will create a check of its own so there's no reason to split it up for that reason.

Or what am I missing?

@ogandose
Copy link
Contributor Author

Yes, this is the whole worklflow. Mostly because we have a lot of dependencies and it takes quite long and we also have dependencies on the internal registry.

@nomego
Copy link
Contributor

nomego commented Sep 26, 2019

I would extend the workflow to include more steps like tests and you'll likely end up in a situation where you need more and more npm stuff and end up starting with an "npm install" and just keeping your deps in there.

But in theory, you don't need to install this annotator with npm install but rather just run:

npx @neovici/github-actions-eslint-annotator@latest

and npm should install whatever is necessary.
I did have some problems with this when I tried but please have a go at it you too.
It would not, however, solve your other installs like additional eslint plugins.

@nomego nomego added this to To do in October 29 - Asterix via automation Sep 26, 2019
@ThiefMaster
Copy link

👍 on >=5

I'm already installing eslint 6 from my package.json during an npm install step and I'm pretty sure my eslint config isn't compatible with eslint 5 anymore...

@github-actions
Copy link

🎉 This issue has been resolved in version 0.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants