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 to Install-ChocolateyZipPackage.ps1 #1732

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

it-praktyk
Copy link

No description provided.

Copy link
Member

@ferventcoder ferventcoder left a comment

Choose a reason for hiding this comment

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

@it-praktyk Great start, just have a couple fix ups requested. Thanks!

@@ -196,14 +199,14 @@ param(

$chocTempDir = $env:TEMP
$tempDir = Join-Path $chocTempDir "$($env:chocolateyPackageName)"
if ($env:chocolateyPackageVersion -ne $null) { $tempDir = Join-Path $tempDir "$($env:chocolateyPackageVersion)"; }
if ($null -ne $env:chocolateyPackageVersion) { $tempDir = Join-Path $tempDir "$($env:chocolateyPackageVersion)"; }
Copy link
Member

Choose a reason for hiding this comment

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

@it-praktyk curious how this is a code smell. Also, the commit message is going to need quite a bit more to explain the change, explain the reasoning, etc. Feel free to start the message with a good summary, but then dive into all of the why in the git commit message.

Copy link
Author

Choose a reason for hiding this comment

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

OK, you are right. I've reworded the commit message.

@@ -71,6 +71,9 @@ likely your script folder. If unzipping to your package folder, the path
will be like
`"$(Split-Path -Parent $MyInvocation.MyCommand.Definition)\\file.exe"`

.PARAMETER SpecificFolder
OPTIONAL - This is a specific directory within zip file to extract.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for putting that in!

Copy link
Member

Choose a reason for hiding this comment

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

We may want to explain what version this is first available in or when it was fixed to work appropriately (the released version of choco where this was known to work) - it worked in earlier code, stopped working for awhile, and then started working again more recently when something was fixed.

Copy link
Member

Choose a reason for hiding this comment

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

#676 came out in 0.10.4

Copy link
Member

Choose a reason for hiding this comment

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

I see you are simply bringing over

.PARAMETER SpecificFolder
OPTIONAL - This is a specific directory within zip file to extract.
to here. That's fine. We can leave out the details if you want.

Copy link
Member

Choose a reason for hiding this comment

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

This was the specific commit that fixed this issue - 3b13869

@it-praktyk it-praktyk force-pushed the Update_Install-ChocolateyZipPackage-1 branch from fc70d5e to 7125a9d Compare February 22, 2020 10:58
@it-praktyk it-praktyk force-pushed the Update_Install-ChocolateyZipPackage-1 branch from 7125a9d to 48cc838 Compare February 22, 2020 11:03
@it-praktyk it-praktyk changed the title Trivial updates to Install-ChocolateyZipPackage.ps1 Updates to Install-ChocolateyZipPackage.ps1 Feb 22, 2020
@CLAassistant
Copy link

CLAassistant commented Jun 8, 2020

CLA assistant check
All committers have signed the CLA.

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.

3 participants