(GH-44) Pass the Source correctly to choco #84
Conversation
Jark
force-pushed
the
pass-in-source-parameter-to-choco
branch
from
3062507 to
05f7b9c
Compare
|
Hi @Jark - prior to accepting this PR, we need to get licensing in place. Please note that we will need to ask you to agree to the licensing as your contribution came before we had a LICENSE file in place. And once you can agree to that, we can move forward with your contribution. |
|
Hi @ferventcoder, Just posted a comment on #70 saying I agree with the apache v2 licence, hope that's enough to move my contribution forward :) |
ferventcoder
changed the title
There was a problem hiding this comment.
This looks really good - take a look at my comments.
Also, if you can rewrite your commit to use a summary and body, that would be great. Use (GH-44) Pass Source to choco correctly as the summary and in the body of the git commit message, describe the issue and what this corrects. For more detail, please see https://github.com/chocolatey/choco/blob/master/CONTRIBUTING.md#prepare-commits
| @@ -292,16 +295,26 @@ function IsPackageInstalled | |||
| } | |||
|
|
|||
| Function Test-LatestVersionInstalled { | |||
| [Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidUsingInvokeExpression','')] | |||
There was a problem hiding this comment.
I added the use of Invoke-Expression to Test-LatestVersionInstalled, since the other internal functions (InstallPackage / Upgrade-Package), seem to use Invoke-Expression as well and that specific PSScriptAnalyzer rule will cause the build to fail.
I don't mind changing all uses of Invoke-Expression to Start-Process in this pull request, but this might be better to do in a different commit, just let me know.
| param( | ||
| [Parameter(Position=0,Mandatory)] | ||
| [string]$pName | ||
| [string]$pName, | ||
| [Parameter(Position=1,Mandatory)] |
There was a problem hiding this comment.
Not a fan of positional parameters - could this be a named resource only?
There was a problem hiding this comment.
Yes, will make this change. Only added the positional parameter bindings because the other functions seemed to use them.
| if ($cParams) { | ||
| $chocoupgradeparams += " $cParams" | ||
| } | ||
| $cmd = "choco upgrade -dv -y $pName $chocoupgradeparams" | ||
| $cmd = "choco upgrade $pName $chocoupgradeparams" |
Jark
force-pushed
the
pass-in-source-parameter-to-choco
branch
4 times, most recently
from
85fa873 to
847b07f
Compare
|
@ferventcoder changes made, hopefully the commit message is in the right format now. Let me know if you want me to replace Invoke-Expression with Start-Process in this DSC resource. P.S. it might be a good idea to add the link to https://github.com/chocolatey/choco/blob/master/CONTRIBUTING.md#prepare-commits to the README.md of this repository, it was very helpful! |
The Source parameter for the cChocoPackageInstaller DSC Resource is not used by the choco install and choco upgrade commands so packages get installed and upgraded from the central chocolatey repository instead of the url specified in the Source parameter. This commit rectifies this issue.
Jark
force-pushed
the
pass-in-source-parameter-to-choco
branch
from
847b07f to
58bd11e
Compare
|
Definitely plan to add more here as we continue to ramp this repository up for Chocolatey. |
|
Thanks for your contribution! |
Hey all,
Hope this pull request is appreciated, I needed the
$Sourceparameter for a demo internally so I modified the module to pass the$Sourceparameter into choco. This fixes issue #44.I've passed this into
choco installandchoco upgrade, I've not added the--sourceparameter tochoco uninstall, since I wasn't sure if it needed it, please advise if that needs it as well and I can add it to that command as well.Note that I've removed the duplicate
-dv -yswitch for thechoco upgradecommand because it's already defined as the start of the command.