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

bugfix: include tslint in deps when a tslintrc exists #325

Merged
merged 2 commits into from Mar 5, 2019

Conversation

43081j
Copy link
Member

@43081j 43081j commented Mar 4, 2019

This should fix part of a few issues like #319.

I introduced tslint as a dependency every time a tslintrc file exists. Let me know if you think this is reasonable (and also if its fine that we still expect it when the tslint config is invalid).

@codecov
Copy link

codecov bot commented Mar 4, 2019

Codecov Report

Merging #325 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #325   +/-   ##
======================================
  Coverage    98.4%   98.4%           
======================================
  Files          32      32           
  Lines         563     563           
======================================
  Hits          554     554           
  Misses          9       9
Impacted Files Coverage Δ
src/special/tslint.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 3190ced...0717082. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 4, 2019

Codecov Report

Merging #325 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #325      +/-   ##
==========================================
+ Coverage   98.41%   98.42%   +<.01%     
==========================================
  Files          32       32              
  Lines         569      570       +1     
==========================================
+ Hits          560      561       +1     
  Misses          9        9
Impacted Files Coverage Δ
src/utils/linters.js 100% <ø> (ø) ⬆️
src/special/eslint.js 100% <100%> (ø) ⬆️
src/special/tslint.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 4a85a2c...a9c9d65. Read the comment docs.

@43081j
Copy link
Member Author

43081j commented Mar 4, 2019

alright so i've made a change to the config file regex matching, too.

it turns out our patterns were kinda wrong..

ESLint supports: .eslintrc* (yaml, json, yml)
TSLint supports tslint.(json|yaml|yml) (there is no concept of .tslintrc etc)

should this be considered a breaking change? as we will no longer support those unsupported filenames.

@rumpl
Copy link
Member

rumpl commented Mar 5, 2019

Huh, weird that we thought that there was a .tslintrc...
We should keep in mind that palantir/tslint#4534

@rumpl
Copy link
Member

rumpl commented Mar 5, 2019

Could you please rebase so that we can merge safely, thanks!

@43081j
Copy link
Member Author

43081j commented Mar 5, 2019

I suspect it may have been supported a very long time ago, or could just be an easy typo as some aged projects do have such a file.

Tslint deprecation is great news. Eslint already supported typescript for a long time now.

Will rebase soon too

@43081j
Copy link
Member Author

43081j commented Mar 5, 2019

@rumpl rebased

@rumpl
Copy link
Member

rumpl commented Mar 5, 2019

Merged 🎉

@rumpl rumpl merged commit 50775c6 into depcheck:master Mar 5, 2019
@43081j 43081j deleted the tslint-fix branch March 5, 2019 08:41
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

2 participants