chocolatey / cChoco

Community resource to manage Chocolatey
Apache License 2.0
154 stars 99 forks source link

(GH-95) Allow for upgrading of packages #134

Open alphakilo45 opened 4 years ago

alphakilo45 commented 4 years ago

This is related to #95 and adds a new resource parameter called UpgradeLowerVersions that causes the cChocoPackageInstall resource to call choco update instead of executing an uninstall/install when the package is already installed and on a lower version.

alphakilo45 commented 4 years ago

I went with a UpgradeLowerVersion parameter to avoid changing how the resource works by default (uninstall/install behavior). I would have expected upgrading packages to be the default behavior as well. Also all your comments should now be addressed @pauby

pauby commented 4 years ago

@alphakilo45 I'm not clear again as to why we are using the parameter UpgradeLowerVersion. I mean I understand why you are using it as you outlined above, but I want to say that Uninstalling and then Installing is not the correct behaviour and it should be a straight upgrade. So that being said we should alter the behaviour? I can't think of any reason why would we not want to?

Thoughts?

alphakilo45 commented 4 years ago

@pauby Only reason I can think of is when the build/pre-release tag is the only difference in the version number and that'll still be supported. I'll get the code updated.

pauby commented 4 years ago

Those tests failed quite spectacularly! Can you rebase and re-push?

alphakilo45 commented 4 years ago

Agreed - rebase didn't help so I need to find some time on keyboard to figure out what is causing that.

alphakilo45 commented 4 years ago

@pauby Looks like Pester released v5 recently and that is causing issues with the tests. I fixed the maxed version in AppVeyor at 4.10.1 (the last pre-5.0.0 version) and tests are now running green.