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

use eslint name normalization #305

Merged
merged 1 commit into from
Jan 23, 2019
Merged

use eslint name normalization #305

merged 1 commit into from
Jan 23, 2019

Conversation

43081j
Copy link
Member

@43081j 43081j commented Jan 22, 2019

It is possible in ESLint to have plugins in the following formats:

[
  "@foo/eslint-plugin",
  "@foo",
  "bar",
  "eslint-plugin-bar"
]

We currently only support the last two in depcheck, which leads to some freaky errors:

Missing dependencies: { 'eslint-plugin-@typescript-eslint/eslint-plugin':

I took the function as-is from eslint and left a jsdoc link to it.

@43081j
Copy link
Member Author

43081j commented Jan 22, 2019

Duplicate of #234 apparently, my mistake.

Though the implementation at #234 doesn't use eslint's own normalisation function and probably should instead of re-inventing the wheel and potentially having different behaviour.

@rumpl
Copy link
Member

rumpl commented Jan 22, 2019

Build failed because the copy/pasted code from eslint makes eslint check fail, I think that's beautiful.

Could you take a look and resolve the issues please?

@codecov
Copy link

codecov bot commented Jan 22, 2019

Codecov Report

Merging #305 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #305      +/-   ##
==========================================
+ Coverage   98.05%   98.11%   +0.06%     
==========================================
  Files          30       30              
  Lines         462      477      +15     
==========================================
+ Hits          453      468      +15     
  Misses          9        9
Impacted Files Coverage Δ
src/utils/linters.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a09a06...19d86eb. Read the comment docs.

@43081j
Copy link
Member Author

43081j commented Jan 22, 2019

sorry about that, the shame of not running lint before pushing 😞

@43081j 43081j force-pushed the scoped-names branch 2 times, most recently from 255d71f to 38e9fd6 Compare January 22, 2019 16:20
@43081j
Copy link
Member Author

43081j commented Jan 23, 2019

@rumpl mind having another look at this?

@rumpl
Copy link
Member

rumpl commented Jan 23, 2019

There are two checks failing for this PR, could you add some more tests so that the codecov is happy please? You can see the code not covered by tests here.

Thanks for contributing! ;)

@43081j
Copy link
Member Author

43081j commented Jan 23, 2019

sorry totally didn't see the check somehow. should be sorted now

@rumpl rumpl merged commit 1375cf2 into depcheck:master Jan 23, 2019
@rumpl
Copy link
Member

rumpl commented Jan 23, 2019

Thanks for this!

I'll try (very hard this time) to review the other PRs and maybe make a new release.

Cheers

@43081j 43081j deleted the scoped-names branch January 23, 2019 17:53
@lijunle
Copy link
Member

lijunle commented Jan 26, 2019

@rumpl it is great that you have a new job and continue on this project! 😋

@rumpl
Copy link
Member

rumpl commented Jan 26, 2019

Thanks @lijunle :)

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.

None yet

3 participants