Publish a StoreBroker NuGet package. #30

danbelcher-MSFT · 2017-03-07T19:29:28Z

This change adds support for publishing a StoreBroker NuGet package. The CONTRIBUTING.md is updated to reference the internal repo as the location
for documentation describing the packaging/publishing process.

StoreBroker.nuspec is added to the root directory and describes the content for the .nupkg. The author of the package is Microsoft and the package will be co-owned by the 'microsoft' and 'nugetstorebrokerteam' NuGet accounts.

The .nupkg will contain the entirety of the StoreBroker subfolder (the files constituting the module), both of the Extensions\ConvertFrom-* scripts, and the XSD files from the PDP folder (for validating PDP XML schema).

Resolves #1 : Create and publish NuGet package

msftclas · 2017-03-07T19:29:32Z

@danbelcher-MSFT,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

HowardWolosky

Good start

HowardWolosky · 2017-03-09T01:47:20Z

CONTRIBUTING.md

@@ -70,6 +70,9 @@ As StoreBroker is a production dependency for Microsoft, we have a couple workfl
  the members above.
 - Releases are performed by a member above so that we can ensure Microsoft internal processes
  remain up to date with the latest and that there are no regressions.
+- Although this repo contains documentation for publishing the StoreBroker NuGet package, that
+  workflow is not able to be completed by employees outside Microsoft. For more information,


"by employees outside Microsoft" -> "by users outside Microsoft"

HowardWolosky · 2017-03-09T01:51:36Z

Extensions/New-NuGetPackage.ps1

+# Copyright (C) Microsoft Corporation.  All rights reserved.
+
+<#
+    .SYNOPSIS


Duplicate the .SYNOPSIS as .DESCRIPTION, but then also add:

The Git-repo for the StoreBroker module can be found here: https://aka.ms/StoreBroker

HowardWolosky · 2017-03-09T01:56:15Z

Extensions/New-NuGetPackage.ps1

+$manifest = Invoke-Expression -Command $manifestContent
+
+# Create the package using the module version
+& nuget pack $NuSpecPath -Properties version=$($manifest.ModuleVersion) -OutputDirectory $OutPath


Doesn't look like nuget is available in the default path -- also possibly not even installed by default. May want to consider dot sourcing Helpers.ps1 and NugetTools.ps1 so that you can access Get-NugetExe to ensure you have access to it without needing to bother the user...

HowardWolosky · 2017-03-09T01:57:02Z

Documentation/NUGET.md

+
+This document describes the process for releasing an updated version of the StoreBroker NuGet package.
+It assumes you are a member of the `[email protected]` distribution list and, therefore,
+is restricted to Microsoft internal employees.


no need for the word "internal"

HowardWolosky · 2017-03-09T01:59:03Z

Documentation/NUGET.md

+3.  At the sign-in page, click `Lost your Password`.
+4.  Send the password reset email to `[email protected]`.
+5.  Open the email (check your spam folder if you do not see it) and follow the reset link.
+6.  The new password will be a GUID. From a PowerShell console, run:


"The new password will be a GUID." --> "Use a GUID for the new password."

HowardWolosky · 2017-03-09T02:04:10Z

Documentation/NUGET.md

+5.  Open the email (check your spam folder if you do not see it) and follow the reset link.
+6.  The new password will be a GUID. From a PowerShell console, run:
+
+        [Guid]::NewGuid()


Could possibly suggest this instead:

(Get-Culture).TextInfo.ToTitleCase([Guid]::NewGuid())

HowardWolosky · 2017-03-09T19:59:13Z

Extensions/New-NuGetPackage.ps1

+        downloads nuget.exe from http://nuget.org to a new local temporary directory
+        and returns the path to the local copy.
+
+        The Git repo for this module can be found here: http://aka.ms/StoreBroker


update link to https

HowardWolosky · 2017-03-09T20:03:47Z

Extensions/New-NuGetPackage.ps1

+        Write-Host "]."
+
+        $out = @()
+        $out += "To prevent this download in the future, download NuGet.exe from NuGet.org and"


instead of saying "download NuGet.exe from NuGet.org and make sure it is accessible from the system PATH", why not just say "copy $nugetExePath to a directory accessible from the system PATH"?

I clean up the temporary directory when the script finishes, in order to not clutter the computer, so the downloaded executable no longer exists. Do you prefer that I leave the temp folder and print this message you suggested?

Right, right. I wrote that comment before I saw your other code later. Leave what you have as-is.

HowardWolosky · 2017-03-09T20:05:50Z

Extensions/New-NuGetPackage.ps1

+    $manifest = Invoke-Expression -Command $manifestContent
+
+    # Create the package using the module version
+    & (Get-NugetExe) pack $NuSpecPath -Properties version=$($manifest.ModuleVersion) -OutputDirectory $OutPath -NoPackageAnalysis


if $OutPath is specified as a relative path, will this still work? Does the path need to be resolved first?

Yes, still works with a relative path

HowardWolosky · 2017-03-09T20:08:38Z

Extensions/New-NuGetPackage.ps1

+    $manifest = Invoke-Expression -Command $manifestContent
+
+    # Create the package using the module version
+    & (Get-NugetExe) pack $NuSpecPath -Properties version=$($manifest.ModuleVersion) -OutputDirectory $OutPath -NoPackageAnalysis


Why don't we want package analysis?

The package analysis dumps a bunch of warnings, none of which are useful, and recommends an action that we don't want. Every .ps1 gets a 3-line warning:
WARNING: Issue: PowerShell file outside tools folder.
WARNING: Description: The script file 'StoreBrokerNuGet\StoreBroker\NugetTools.ps1' is outside the 'tools' folder and hence will not be executed during installation of this package.
WARNING: Solution: Move it into the 'tools' folder.

Hmmm...tried doing some searching to see if there's something that can be done to mark those as ignorable and still get the analysis, but came up short. It's probably worth a comment there to indicate that analysis is skipped because it gives false-positive warnings since we're not putting our PowerShell scripts into the Tools directory.

HowardWolosky · 2017-03-09T20:09:53Z

Extensions/New-NuGetPackage.ps1

+}
+finally
+{
+    if (($null -ne $script:tempFolderPath) -and (Test-Path -PathType Container -Path $script:tempFolderPath))


Add a comment that this is being set in Get-NugetExe, and then add a .NOTES section to Get-NugetExe that a side-effect of running that script is the potential setting of that script variable. That script variable should also be set to $null by default for clarity.

HowardWolosky · 2017-03-09T20:12:50Z

StoreBroker/NugetTools.ps1

@@ -30,7 +30,7 @@ function Get-NugetExe
        $sourceNugetExe = "https://dist.nuget.org/win-x86-commandline/latest/nuget.exe"
        $script:nugetExePath = Join-Path $(New-TemporaryDirectory) "nuget.exe"

-        Write-Log "Downlading $sourceNugetExe to $script:nugetExePath" -Level Verbose
+        Write-Log "Downloading $sourceNugetExe to $script:nugetExePath" -Level Verbose


Thanks for the fix. Since you are changing a file in the module though, increment the patch number in the module manifest.

HowardWolosky · 2017-03-09T20:45:26Z

Extensions/New-NuGetPackage.ps1

+        $sourceNugetExe = "https://dist.nuget.org/win-x86-commandline/latest/nuget.exe"
+        $nugetExePath = Join-Path -Path $script:tempFolderPath -ChildPath "nuget.exe"
+
+        Write-Host "Downloading [" -NoNewline


This seems a bit unnecessary. I tried it and didn't get a lot of value out of it, visually.
Just stick with a regular, old-fashioned single line Write-Host without bothering with ForegroundColor. It'll make the code easier t read that way too. Skip the coloring on the next one too.

danbelcher-MSFT · 2017-03-14T23:38:36Z

StoreBroker/StoreBroker.psd1

@@ -6,7 +6,7 @@
    CompanyName = 'Microsoft Corporation'
    Copyright = 'Copyright (C) Microsoft Corporation.  All rights reserved.'

-    ModuleVersion = '1.4.5'
+    ModuleVersion = '1.4.6'


Will update ModuleVersion appropriately when it's time to check in

HowardWolosky · 2017-03-16T19:13:14Z

Extensions/New-NuGetPackage.ps1

+# New-NuGetPackage.ps1 body
+try
+{
+    if (-not $IgnoreUnsigned.IsPresent)


Interesting. I wasn't aware of the IsPresent property on switches. I think that could be useful in the future.

HowardWolosky · 2017-03-16T19:14:51Z

Documentation/NUGET.md

+    Every time you need to access the account you should follow the password reset process.
+7.  Sign in using the new password.
+
+### Create an API Key


Will you be updating this with publishing steps as well?

HowardWolosky · 2017-03-16T19:16:20Z

Documentation/NUGET.md

+
+This document describes the process for releasing an updated version of the StoreBroker NuGet package.
+
+### Signing


This should note that the signed files should be pushed into master as well.

HowardWolosky

Looks good. Go ahead and fix these remaining changes, do a final rebase/squash, and then bring it in to Master.

Thanks!

HowardWolosky · 2017-04-19T19:48:04Z

CONTRIBUTING.md

@@ -301,6 +301,9 @@ Where:
 * `<minor>` - If this is a feature update, increment by one and be sure to reset `<patch>` to 0.
 * `<patch>` - If this is a bug fix, leave `<minor>` alone and increment this by one.

+When new code changes are checked in to the repo, a new NuGet package must be published. This process is documented in


must be published by Microsoft.

HowardWolosky · 2017-04-19T19:48:30Z

CONTRIBUTING.md

@@ -301,6 +301,9 @@ Where:
 * `<minor>` - If this is a feature update, increment by one and be sure to reset `<patch>` to 0.
 * `<patch>` - If this is a bug fix, leave `<minor>` alone and increment this by one.

+When new code changes are checked in to the repo, a new NuGet package must be published. This process is documented in
+the internal repo. Refer to the CONTRIBUTING.md in that repo for more information on creating a signed NuGet package.


documented in Microsoft's internal StoreBroker repo.

HowardWolosky · 2017-04-19T19:49:53Z

StoreBroker.nuspec

+  </metadata>
+  <files>
+      <file src="StoreBroker\" target="StoreBroker\"/>
+      <file src="Extensions\ConvertFrom-ExistingSubmission.ps1" target="Extensions\"/>


Curious ... why call out these individual files here as opposed to just having all extensions go to Extensions the way that you do for StoreBroker\?

Ah, this was because an earlier iteration introduced a script here that we didn't want packaged. That script is moved to the internal repo so it will be fine to just have the Extensions folder. Good feedback.

HowardWolosky · 2017-04-19T19:51:11Z

StoreBroker/StoreBroker.psd1

@@ -6,7 +6,7 @@
    CompanyName = 'Microsoft Corporation'
    Copyright = 'Copyright (C) Microsoft Corporation.  All rights reserved.'

-    ModuleVersion = '1.6.1'
+    ModuleVersion = '1.7.0'


You'll need to update this again. Just update the patch version though (so 1.7.1).

This change adds support for publishing a StoreBroker NuGet package. The CONTRIBUTING.md is updated to reference the internal repo as the location for documentation describing the packaging/publishing process. StoreBroker.nuspec is added to the root directory and describes the content for the .nupkg. The author of the package is Microsoft and the package will be co-owned by the 'microsoft' and 'nugetstorebrokerteam' NuGet accounts. The .nupkg will contain the entirety of the StoreBroker subfolder (the files constituting the module), both of the Extensions\ConvertFrom-* scripts, and the XSD files from the PDP folder (for validating PDP XML schema).

danbelcher-MSFT requested a review from HowardWolosky March 7, 2017 19:29

msftclas added the cla-not-required label Mar 7, 2017

HowardWolosky requested changes Mar 9, 2017

View reviewed changes

danbelcher-MSFT commented Mar 14, 2017

View reviewed changes

HowardWolosky requested changes Mar 16, 2017

View reviewed changes

HowardWolosky approved these changes Apr 19, 2017

View reviewed changes

danbelcher-MSFT merged commit 3ad0efa into microsoft:master Apr 19, 2017

danbelcher-MSFT deleted the forked/NuGetPackaging branch April 19, 2017 22:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Publish a StoreBroker NuGet package. #30

Publish a StoreBroker NuGet package. #30

danbelcher-MSFT commented Mar 7, 2017 •

edited

Loading

msftclas commented Mar 7, 2017

HowardWolosky left a comment

HowardWolosky Mar 9, 2017

HowardWolosky Mar 9, 2017

HowardWolosky Mar 9, 2017

HowardWolosky Mar 9, 2017

HowardWolosky Mar 9, 2017

HowardWolosky Mar 9, 2017

HowardWolosky Mar 9, 2017

HowardWolosky Mar 9, 2017

danbelcher-MSFT Mar 9, 2017

HowardWolosky Mar 9, 2017

HowardWolosky Mar 9, 2017

danbelcher-MSFT Mar 9, 2017

HowardWolosky Mar 9, 2017

danbelcher-MSFT Mar 9, 2017

HowardWolosky Mar 9, 2017

HowardWolosky Mar 9, 2017

HowardWolosky Mar 9, 2017

HowardWolosky Mar 9, 2017

danbelcher-MSFT Mar 9, 2017

danbelcher-MSFT Mar 14, 2017

HowardWolosky Mar 16, 2017

HowardWolosky Mar 16, 2017

HowardWolosky Mar 16, 2017

HowardWolosky left a comment

HowardWolosky Apr 19, 2017

HowardWolosky Apr 19, 2017

HowardWolosky Apr 19, 2017

danbelcher-MSFT Apr 19, 2017

HowardWolosky Apr 19, 2017


		This document describes the process for releasing an updated version of the StoreBroker NuGet package.

		### Signing

Publish a StoreBroker NuGet package. #30

Publish a StoreBroker NuGet package. #30

Conversation

danbelcher-MSFT commented Mar 7, 2017 • edited Loading

msftclas commented Mar 7, 2017

HowardWolosky left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HowardWolosky left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danbelcher-MSFT commented Mar 7, 2017 •

edited

Loading