The Wayback Machine - https://web.archive.org/web/20200904162907/https://github.com/microsoft/WinObjC/pull/2233
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

Fix chocolatey install/ uninstall for vs2017 #2233

Merged
merged 4 commits into from Mar 14, 2017

Conversation

@bbowman
Copy link
Member

bbowman commented Mar 14, 2017

No description provided.

@bbowman
Copy link
Member Author

bbowman commented Mar 14, 2017

Copy link

DHowett-MSFT left a comment

We should depend on vswhere as a chocolatey package instead of shipping an ever-unchanging binary.

Also, what happened with ThirdPartyNotices? It looks like it went through an industrial-scale shredder.

s
@bbowman bbowman force-pushed the bbowman:bbowman/fix_tools_install branch from 7c67179 to e049bbc Mar 14, 2017
<PackageReleaseNotes>
</PackageReleaseNotes>
<PackageProjectUrl>https://github.com/Microsoft/WinObjC</PackageProjectUrl>
<PackageLicenseUrl>
</PackageLicenseUrl>
<PackageLicenseUrl>https://github.com/Microsoft/WinObjC/blob/develop/license.txt</PackageLicenseUrl>

This comment has been minimized.

@memontic-ms

memontic-ms Mar 14, 2017

Member

develop [](start = 65, length = 7)

This develop's license, not master's. IMO probably want to have the license that our releases will be associated with. They are currently the same though.

This comment has been minimized.

@bbowman

bbowman Mar 14, 2017

Author Member

I was thinking develop since it will be the latest always. In case we have changes, this will most quickly reflect any license changes but I honestly don't know which is better.


In reply to: 106000391 [](ancestors = 106000391)

This comment has been minimized.

@memontic-ms

memontic-ms Mar 14, 2017

Member

@rajsesh-msft


In reply to: 106002170 [](ancestors = 106002170,106000391)

<Summary>WinObjC.Tools</Summary>
<Description>WinObjC.Tools</Description>
<Summary>Command Line tools and Visual Studio Extensions for the WinObjC Project.</Summary>
<Description>Command Line tools and Visual Studio Extensions for the WinObjC Project. This package includes vsimporter, the tool to convert an existing xcode objective c project to a visual studio solution, as well as the Visual Studio Extensions for Objective C syntax highlighting and Nuget package creation. See https://github.com/Microsoft/WinObjC for more details.</Description>
<PackageReleaseNotes>
</PackageReleaseNotes>

This comment has been minimized.

@memontic-ms

memontic-ms Mar 14, 2017

Member

We should note here somewhere that this package is for VS 2017. Maybe compatibility notes specifically go in the PackageReleaseNotes section? At the very least change occurrences of Visual Studio to Visual Studio 2017 in the summary & description.

This comment has been minimized.

@bbowman

bbowman Mar 14, 2017

Author Member

its 2017 or later so I didn't want to be too specific. I'll add a line to description though.

<Summary>WinObjC.Tools</Summary>
<Description>WinObjC.Tools</Description>
<Summary>Command Line tools and Visual Studio Extensions for the WinObjC Project.</Summary>
<Description>Command Line tools and Visual Studio Extensions for the WinObjC Project. This package includes vsimporter, the tool to convert an existing xcode objective c project to a visual studio solution, as well as the Visual Studio Extensions for Objective C syntax highlighting and Nuget package creation. See https://github.com/Microsoft/WinObjC for more details.</Description>

This comment has been minimized.

@memontic-ms

memontic-ms Mar 14, 2017

Member

visual studio [](start = 187, length = 13)

nit: Capitalize Visual Studio

# despite the name "Install-ChocolateyZipPackage" this also works with 7z archives
#Install-ChocolateyZipPackage "$packageName" "$url" "$installDir" "$url64"
# END PORTABLE EXAMPLE
if($result -gt 0 -and $result -ne 1001) { #1001: Already installed

This comment has been minimized.

@memontic-ms

memontic-ms Mar 14, 2017

Member

#1001: Already installed [](start = 42, length = 24)

Wouldn't hurt to Write-Host the already installed case.
Also, is there any indication that an install was successful besides it not failing? Is there Chocolatey magic that does that?

This comment has been minimized.

@bbowman

bbowman Mar 14, 2017

Author Member

Following the Install-ChocoalteyVsix pattern as much as possible here which doesn't. #wontfix

@bbowman bbowman requested a review from pradipd Mar 14, 2017
Copy link

ditta95aR left a comment

@bbowman
Copy link
Member Author

bbowman commented Mar 14, 2017

@DHowett-MSFT updated.

@bbowman bbowman merged commit b4f987f into microsoft:packaging Mar 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.