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

[Updates] Remove 'volta activate' and 'volta deactivate' commands #559

Merged
merged 2 commits into from
Nov 12, 2019

Conversation

charlespierce
Copy link
Contributor

@charlespierce charlespierce commented Oct 9, 2019

Related to #222
Closes #562
Closes #216
Closes #165
Closes #104
Closes #99

Info

  • As part of the push towards automatic updates, we want to enable Volta to work directly on the command line, instead of through a shell function wrapper.
  • The purpose of the function wrapper was to allow the volta activate and volta deactivate commands to work as expected, modifying the PATH environment variable for the current shell.
  • However, those commands were intended as a release valve for when we were in early testing phases, and are no longer needed since Volta is much more stable now.

Changes

  • Removed the activate and deactivate commands from the Volta CLI and the associated ActivityType
  • Moved the VOLTA_UNSAFE_GLOBAL environment variable definition to the run module, where it is used.
  • Removed the shell and env modules, which previous existed primarily to support the Postscript behavior that is no longer needed without activate / deactivate
  • Removed System::enabled_path, which was only used by volta activate before.
  • Deleted tests that referenced no-longer-available blocks / commands.

Tested

  • Verified that volta activate and volta deactivate are no longer allowed commands
  • All automated tests pass

Notes

  • This PR builds on top of [volta updates] Add volta-updates feature and CI override #558, as it uses the feature flag added in that PR, so it will remain a draft until that is merged.
  • Since this is connected to the automatic updates, all of the changes are locked behind the volta-updates feature flag, so "removal" generally meant adding a #[cfg(not(feature = "volta-updates"))] attribute to the relevant block / line of code.

@charlespierce charlespierce marked this pull request as ready for review October 9, 2019 23:05
@thoov
Copy link
Member

thoov commented Oct 15, 2019

@charlespierce Should we deprecate these first? I know they aren't public but might be a good practice to either version bump this to 0.7.0 or include at least one version with a warning deprecation before removing

@charlespierce
Copy link
Contributor Author

@thoov Yeah, that's a great idea. This change is locked behind the updates feature flag and won't roll out until we're ready to have that entire feature, which will definitely be a 0.7.0 release, but I'll make another PR that adds a deprecation warning to volta activate and volta deactivate for the 0.6.x releases.

@charlespierce
Copy link
Contributor Author

Created #571 to add the deprecation warnings to the current block

@charlespierce charlespierce changed the title Remove 'volta activate' and 'volta deactivate' commands [Updates] Remove 'volta activate' and 'volta deactivate' commands Oct 21, 2019
Copy link
Contributor

@chriskrycho chriskrycho left a comment

Choose a reason for hiding this comment

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

Ship it!

@chriskrycho chriskrycho merged commit 7dc46ad into volta-cli:master Nov 12, 2019
@charlespierce charlespierce deleted the remove_deactivate branch December 3, 2019 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants