Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

fix: update update-notifier #2212

Merged
merged 1 commit into from
Feb 17, 2017
Merged

Conversation

joscha
Copy link
Contributor

@joscha joscha commented Feb 17, 2017

PR checklist

  • New feature, bugfix, or enhancement

What changes did you make?

Currently tslint dependes on update-notifier 1.x, unfortunately the dependency chain of update-notifier means that at the very end got@5.7.1 is being pulled in:

├─┬ tslint@4.4.2
│ ├── diff@3.2.0
│ ├─┬ findup-sync@0.3.0
│ │ └── glob@5.0.15
│ ├─┬ optimist@0.6.1
│ │ ├── minimist@0.0.10
│ │ └── wordwrap@0.0.3
│ ├── resolve@1.2.0
│ └─┬ update-notifier@1.0.3
│   ├─┬ boxen@0.6.0
│   │ ├── ansi-align@1.1.0
│   │ ├── camelcase@2.1.1
│   │ ├── cli-boxes@1.0.0
│   │ ├── filled-array@1.1.0
│   │ └── widest-line@1.0.0
│   ├── is-npm@1.0.0
│   ├─┬ latest-version@2.0.0
│   │ └─┬ package-json@2.4.0
│   │   ├─┬ got@5.7.1
│   │   │ ├─┬ create-error-class@3.0.2
│   │   │ │ └── capture-stack-trace@1.0.0

got@5.7.1 again has an engines restriction of node<7, which means that if you install tslint with node 7 and engine-strict=true, you get an error:

npm ERR! notsup Unsupported engine for got@5.7.1: wanted: {"node":">=0.10.0 <7"} (current: {"node":"7.5.0","npm":"3.10.7"})
npm ERR! notsup Not compatible with your version of node/npm: got@5.7.1
npm ERR! notsup Not compatible with your version of node/npm: got@5.7.1
npm ERR! notsup Required: {"node":">=0.10.0 <7"}
npm ERR! notsup Actual:   {"npm":"3.10.7","node":"7.5.0"}

This PR just updates the update-notifier dependency, which in that whole chain then allows got@6.x which supports Node 7.

@palantirtech
Copy link
Member

Thanks for your interest in palantir/tslint, @joscha! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@adidahiya adidahiya merged commit f41ad5b into palantir:master Feb 17, 2017
@adidahiya
Copy link
Contributor

thanks @joscha. heads up, we're considering removing this dependency because it's pretty heavy (#2151)

@joscha
Copy link
Contributor Author

joscha commented Feb 18, 2017

No worries, thanks for merging - I was just interested in getting it to install under node 7. Will a new version automatically be released?

@joscha joscha deleted the update-update-notifier branch February 18, 2017 01:13
@joscha
Copy link
Contributor Author

joscha commented Feb 21, 2017

@adidahiya would you mind pushing out a new patch release with the changes, please?

@agjs
Copy link

agjs commented Feb 25, 2017

@adidahiya Pushing this as soon as possible would save time to many people, including myself. Thanks

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants