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

(#2468) Remove deprecated Chocolatey commands #2550

Merged
merged 2 commits into from
Mar 14, 2022

Conversation

gep13
Copy link
Member

@gep13 gep13 commented Jan 22, 2022

The following Chocolatey commands have been marked as deprecated for a while now, and with the upcoming release of 1.0.0, it is time to remove them:

  • choco version
  • choco update

The functionality of these commands is now provided elsewhere, and these commands can be removed.

In addition, the shim for cver has also been removed.

Fixes #2468

https://app.clickup.com/t/20540031/ENGTASKS-723

@gep13 gep13 requested review from AdmiringWorm and corbob January 22, 2022 15:35
@corbob
Copy link
Member

corbob commented Feb 1, 2022

Should there be an upgrade path that removed the cver.exe shim?

@gep13
Copy link
Member Author

gep13 commented Feb 1, 2022

@corbob also not sure what you mean here, can you elaborate?

@corbob
Copy link
Member

corbob commented Feb 1, 2022

So if I take the nupkg to a fresh system and install it, cver.exe is not there. But if I already have Chocolatey on the box and do choco upgrade chocolatey cver.exe is still on the path.

Copy link
Member

@corbob corbob left a comment

Choose a reason for hiding this comment

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

Aside from the previous comment about possibly removing cver.exe on upgrade, it all looks good to me. My testing shows it all works as expected both from an upgrade and from a fresh install perspective.

The removal can be discussion and a separate PR if we decide that's something for us to do.

@pauby
Copy link
Member

pauby commented Feb 3, 2022

That's a good point @corbob.

@gep13 it looks like the shims are not created when Chocolatey is installed for the first time but doesn't remove it on upgrade? Would there have to be changes to the install script?

@gep13
Copy link
Member Author

gep13 commented Feb 4, 2022

@pauby said...
it looks like the shims are not created when Chocolatey is installed for the first time but doesn't remove it on upgrade?

Just to make sure that are on the same page, I am reading this as:

it looks like the shims are created when Chocolatey is installed for the first time but doesn't remove it on upgrade?

If so, then yes, we need to add some steps to the chocolateyInstall.ps1 file to ensure that the cver shim file is removed from the bin folder when upgrading.

To illustrate what happens...

With version 0.12.1 of Chocolatey installed, attempting to run cver results in the following:

image

When attempting to run it with the code from this PR, you get the following:

image

This is "bad" since the cver shim is still there, which is happening to run choco version and that command is no longer registered.

When we remove the cver shim, we then see the following:

image

Which is what we actually want to see.

I will attempt to update this PR at some point to include the work to ensure that the cver shim is removed (unless someone beats me to this).

@gep13 gep13 marked this pull request as draft February 4, 2022 09:39
@gep13
Copy link
Member Author

gep13 commented Feb 4, 2022

I am converting this to a draft PR until the removal of the shim is completed.

@gep13 gep13 changed the title Remove deprecated Chocolatey commands (#2468) Remove deprecated Chocolatey commands Feb 4, 2022
Copy link
Member

@AdmiringWorm AdmiringWorm left a comment

Choose a reason for hiding this comment

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

This overall looks good to me.

The only thing remaining is to remove the shim during upgrade.
The code for this is included in the PR #2551 and the only change needed for this PR would be to add the cver executable to the list of shims to remove.

gep13 added 2 commits March 14, 2022 16:14
This command is no longer required, and has been replaced by either the
choco list or the choco upgrade --noop commands depending on whether you
are attempting to get locally installed items, or checking to see what
upgrades are available.

In addition, the cver shim has also been removed.
This command has been replaced by the choco upgrade command. At some
point, the choco update command will be re-instated, with the purpose of
updating the local package index of packages.
@gep13 gep13 marked this pull request as ready for review March 14, 2022 16:21
Copy link
Member

@AdmiringWorm AdmiringWorm left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@AdmiringWorm AdmiringWorm merged commit ed34ee7 into chocolatey:develop Mar 14, 2022
@AdmiringWorm
Copy link
Member

@gep13 Thanks for getting this added 👍

@gep13 gep13 deleted the issue-2468 branch March 14, 2022 17:01
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.

Remove deprecated Chocolatey commands and shims
4 participants