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

fix(gatsby-cli): Fixed incorrect scriptname in gatsby-cli #12186

Conversation

yogeshkotadiya
Copy link
Contributor

@yogeshkotadiya yogeshkotadiya commented Feb 28, 2019

Description

Resolves incorrect script name in output when using gatsby --help command.
Also removed yargs configuration from package.json because of this warning
Configuring yargs through package.json is deprecated and will be removed in the next major release, please use the JS API instead.

TO-DO

  • Resolve any Breaking changes due to a semver major upgrade.

Packages upgraded

yargs 11.1.0 --> 13.2.1 --> 12.0.5

Related Issues

Addresses #12154

@yogeshkotadiya yogeshkotadiya requested a review from a team as a code owner February 28, 2019 18:51
DSchau
DSchau previously requested changes Feb 28, 2019
Copy link
Contributor

@DSchau DSchau left a comment

Choose a reason for hiding this comment

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

@yogeshkotadiya it looks like there are a few breaking changes with yargs 13, notably dropping Node 6 https://github.com/yargs/yargs/blob/master/CHANGELOG.md#breaking-changes

Could you remove the upgrade here, and just encapsulate the changes to the incorrect script name?

@DSchau DSchau added the status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response label Mar 5, 2019
Copy link
Contributor

@sidharthachatterjee sidharthachatterjee left a comment

Choose a reason for hiding this comment

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

Left a small comment! Thank you for this, @yogeshkotadiya 👍

@@ -28,7 +28,7 @@
"source-map": "^0.5.7",
"stack-trace": "^0.0.10",
"update-notifier": "^2.3.0",
"yargs": "^11.1.0",
"yargs": "12.0.5",
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is also a major semver upgrade, are there any breaking changes we should resolve? Also, should we add a ^ as opposed to pinning the version?

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"yargs": "12.0.5",
"yargs": "^11.1.0",

Would probably be better to not change the version at all in this PR, what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just saw that scriptName is 12+

@sidharthachatterjee
Copy link
Contributor

Thank you so much, @yogeshkotadiya 👍

@sidharthachatterjee sidharthachatterjee merged commit 3b116f6 into gatsbyjs:master Mar 8, 2019
@sidharthachatterjee
Copy link
Contributor

Published in gatsby-cli@2.4.14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants