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.

Sign up for GitHub

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

Jump to bottom

Support volta and toolchain keys side by side. #434

Merged
merged 4 commits into from
May 16, 2019
Merged

Support volta and toolchain keys side by side. #434

merged 4 commits into from
May 16, 2019

Conversation

chriskrycho
Copy link
Contributor

@chriskrycho chriskrycho commented May 14, 2019 •
edited
Loading

deprecation output 
$ yarn
Volta warning: the `toolchain` key is deprecated and will be removed in a future
               version. Please switch to `volta` instead.
yarn install v1.13.0
[1/4] 🔍  Resolving packages...
success Already up-to-date.
✨  Done in 0.33s.

$ volta pin [email protected]
warning: the `toolchain` key is deprecated and will be removed in a future
         version. Please switch to `volta` instead.
success: pinned [email protected] in package.json

$ volta pin yarn
warning: this project is configured with both the deprecated `toolchain` key and
         the `volta` key; using the versions specified in `volta`.
success: pinned [email protected] in package.json

$ yarn
Volta warning: this project is configured with both the deprecated `toolchain`
               key and the `volta` key; using the versions specified in `volta`.
yarn install v1.16.0
[1/4] 🔍  Resolving packages...
success Already up-to-date.
✨  Done in 0.28s.

# edit package.json to remove `toolchain`

$ yarn
yarn install v1.16.0
[1/4] 🔍  Resolving packages...
success Already up-to-date.
✨  Done in 0.34s.
deprecation output screenshot actual-output

rwjblue
rwjblue reviewed May 14, 2019
View reviewed changes
Copy link
Contributor

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

The screenshots look great! I only had one suggestion (to let the user know where to look to update to fix the deprecation), which might not be possible.

Thanks for jumping on this so quickly!

crates/volta-core/src/manifest/serial.rs Outdated Show resolved Hide resolved
crates/volta-core/src/manifest/serial.rs Outdated Show resolved Hide resolved
@chriskrycho
Copy link
Contributor Author

Thanks for the feedback, @rwjblue! Error messages now read—

  • on finding only toolchain:

    $ volta pin yarn
warning: this project (`/Users/ckrycho/dev/oss/true-myth/package.json`) is
         configured with the `toolchain` key, which is deprecated and will be
         removed in a future version. Please switch to `volta` instead.
success: pinned [email protected] in package.json

  • on finding both toolchain and volta:

    $ volta pin yarn
warning: this project (`/Users/ckrycho/dev/oss/true-myth/package.json`) is
     configured with both the deprecated `toolchain` key and the `volta`
     key; using the versions specified in `volta`.
success: pinned [email protected] in package.json

@rwjblue
Copy link
Contributor

rwjblue commented May 14, 2019

Awesome!

@rwjblue
Copy link
Contributor

rwjblue commented May 14, 2019

Note that due to #431 we don't get CI runs for this yet.

@chriskrycho chriskrycho force-pushed the back-compat-toolchain branch from 55ea535 to 3635705 Compare May 15, 2019 15:20
@chriskrycho chriskrycho force-pushed the back-compat-toolchain branch from 21809cb to 5295d1a Compare May 16, 2019 20:26
charlespierce
charlespierce approved these changes May 16, 2019
View reviewed changes
Copy link
Contributor

@charlespierce charlespierce left a comment

Choose a reason for hiding this comment

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

LGTM!

@charlespierce charlespierce merged commit 5d6cae4 into master May 16, 2019
@chriskrycho chriskrycho deleted the back-compat-toolchain branch May 16, 2019 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rwjblue rwjblue rwjblue left review comments

@charlespierce charlespierce charlespierce approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Do not use into_ for functions which borrow rather than move self Toolchains key no longer recognized
3 participants
@chriskrycho @rwjblue @charlespierce