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

Packages and lint #163

Merged
merged 2 commits into from Jul 27, 2018
Merged

Packages and lint #163

merged 2 commits into from Jul 27, 2018

Conversation

grantwwu
Copy link
Contributor

Bump some packages to fix security issues. I skimmed the changelogs and didn't notice anything that would cause an issue. Tests still pass.

Fix some tslint deprecations.

.gitignore Outdated
@@ -4,7 +4,6 @@ coverage
dist
node_modules
npm-debug.log
package-lock.json
Copy link
Contributor

Choose a reason for hiding this comment

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

It is Apollo convention to exclude lock files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you know why that's the case? Interested in learning more.

Anyways I am willing to drop this though.

Copy link
Contributor

Choose a reason for hiding this comment

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

With common use of npm and yarn it can lead to odd issues unless you are aware of the lock files. Additionally with semver there is no reason not to pickup bug fixes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. I've dropped the commit from my branch.

@NeoPhi NeoPhi merged commit 4e23840 into apollographql:master Jul 27, 2018
@NeoPhi
Copy link
Contributor

NeoPhi commented Jul 27, 2018

Thank you!

@grantwwu
Copy link
Contributor Author

grantwwu commented Jul 28, 2018

@NeoPhi I actually just realized that some of the changes I made deprecations wise are for fairly new versions.

In particular, no-unused-variable was deprecated only 11 days ago: https://github.com/palantir/tslint/releases/tag/5.11.0

The --type-check flag was deprecated/removed in Oct 2017:
https://github.com/palantir/tslint/releases/tag/5.8.0

Moreover neither our tslint nor typescript dependency bounds are strict enough to pull these in.

Do you think we should revert those changes for some specified period of time?

@NeoPhi
Copy link
Contributor

NeoPhi commented Jul 28, 2018

I'm pulling in the correct versions on a fresh install. As this is only relates to devDependencies we should just update the minimum version for tslint.

@grantwwu grantwwu mentioned this pull request Jul 30, 2018
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