Adding windows server names to the windows packages + Misc formatting fixes by HemantMahawar · Pull Request #2757 · PowerShell/PowerShell

HemantMahawar · Nov 22, 2016

Misc fixes are:

Added Get-PackageSemanticVersion function
Added New-ZipPackage function to separate the Zip packaging code
Removed ending ';' in hashtable syntax since entries are on new lines
Standardized the variable name from $Source to $PackageSourcePath for various packaging functions
Removed 'return' in the function end to output the variable value
Added -verbose to Write-Verbose to show the paths of various packages

Addresses #2468

… fix Misc fixes are: - Added Get-PackageSemanticVersion function - Added New-ZipPackage function to separate the Zip packaging code - Removed ending ';' in hashtable syntax since entries are on new lines - Standardized the variable name from $Source to $PackageSourcePath for various packaging functions - Removed 'return' in the function end to output the variable value - Added -verbose to Write-Verbose to show the paths of various packages

HemantMahawar · Nov 22, 2016

@mirichmo and @raghushantha, can you take a look at this PR?

… fix Misc fixes are: - Added Get-PackageSemanticVersion function - Added New-ZipPackage function to separate the Zip packaging code - Removed ending ';' in hashtable syntax since entries are on new lines - Standardized the variable name from $Source to $PackageSourcePath for various packaging functions - Removed 'return' in the function end to output the variable value - Added -verbose to Write-Verbose to show the paths of various packages

… into HM-Patch3

mirichmo · Nov 30, 2016

-            $NameSuffix = $VersionTokens[1], $Runtime -join "-"
+        # Add the server name to the $RunTime. $runtime produced by dotnet is same for client or server
+        switch ($Runtime) {
+            'win81-x64' {$NameSuffix = 'win81-w2k12r2-x64'}


I added support for win7-x86 and win7-x64 in alpha.13. Please expand your change to support those runtimes as well

mirichmo · Nov 30, 2016

+
+    if (3 -eq $packageVersionTokens.Count) {
+        # In case the input is of the form a.b.c, add a '0' at the end for revision field
+        $packageSemanticVersion = $packageVersion,'0' -join '.'


Where does $packageVersion come from? Should it be $Version?

mirichmo · Nov 30, 2016

+        # We have all the four fields
+        $packageRevisionTokens = ($packageVersionTokens[3].Split('-'))[0]
+        $packageSemanticVersion = $packageVersionTokens[0],$packageVersionTokens[1],$packageVersionTokens[2],$packageRevisionTokens -join '.'
+    }


What about the case where 3 -gt $packageVersionTokens.Count? It should throw since $packageSemanticVersion will not be defined.

mirichmo · Nov 30, 2016

+
+    $ProductSemanticVersion = Get-PackageSemanticVersion -Version $PackageVersion
+
+    $PackageVersion = Get-PackageVersionAsMajorMinorBuildRevision -Version $PackageVersion


This is unused except in the Write-Verbose. The actual package name uses $ProductSemanticVesion

@mirichmo Not sure of the ask. Do you want me to remove the variable or call to write-verbose or something else?

I'm not sure of the coding intent here. Why doesn't the Write-Verbose use $ProductSemanticVersion? $PackageVersion is not used anywhere except for the Write-Verbose, so that makes me think it was an oversight when refactoring. Is it intentional?

…o HM-Patch3

mirichmo · Dec 7, 2016

+        switch ($Runtime) {
+            'win81-x64' {$NameSuffix = 'win81-win2k12r2-x64'}
+            'win10-x64' {$NameSuffix = 'win10-win2k16-x64'}
+            'win7-x64'  {$NameSuffix = 'win7-win2k8r2-x64'}


Please add win7-x86 here too and throughout

Given, there is no x86 version of w2K8r2, win7-x86 case should be covered by the Default {$NameSuffix = $Runtime} below, right?

Ahh. Good point

mirichmo · Dec 7, 2016

+        $packageRevisionTokens = ($packageVersionTokens[3].Split('-'))[0]
+        $packageSemanticVersion = $packageVersionTokens[0],$packageVersionTokens[1],$packageVersionTokens[2],$packageRevisionTokens -join '.'
+    } else {
+        throw "Cannot create semantic version from a string $Version containing more than 4 tokens"


How about this instead:
"Cannot create Semantic Version from the string $Version containing 4 or more tokens."

Hemant Mahawar added 2 commits November 21, 2016 15:35

update

e9c53eb

msftclas added the cla-already-signed label Nov 22, 2016

Hemant Mahawar added 3 commits November 21, 2016 16:28

update

0497834

Merge branch 'HM-Patch3' of https://github.com/HemantMahawar/PowerShell…

2edf5cf

… into HM-Patch3

mirichmo self-assigned this Nov 22, 2016

PowerShellTeam added the Review - Needed label Nov 22, 2016

mirichmo added this to the 6.0.0-alpha.14 milestone Nov 22, 2016

PowerShellTeam added the Review - Needed The PR is being reviewed label Nov 22, 2016

mirichmo suggested changes Nov 30, 2016

View reviewed changes

mirichmo added Review-Waiting on Author and removed Review - Needed The PR is being reviewed Review-Needed labels Nov 30, 2016

Hemant Mahawar added 5 commits December 1, 2016 12:14

Merge branch 'master' of https://github.com/PowerShell/PowerShell int…

7c53d79

…o HM-Patch3

Merge branch 'master' of https://github.com/PowerShell/PowerShell int…

f9bd50e

…o HM-Patch3

Addressing review feedback

b851329

Merge branch 'master' of https://github.com/PowerShell/PowerShell int…

f875f43

…o HM-Patch3

Removed the $PackageVersion variable per review feedback

94a8a56

PowerShellTeam added Review - Needed The PR is being reviewed and removed Review - Waiting on Author labels Dec 6, 2016

mirichmo suggested changes Dec 7, 2016

View reviewed changes

Error message fix

9455f76

mirichmo closed this Dec 9, 2016

mirichmo reopened this Dec 9, 2016

msftclas added the cla-already-signed label Dec 9, 2016

mirichmo approved these changes Dec 14, 2016

View reviewed changes

mirichmo merged commit e26c2a8 into PowerShell:master Dec 14, 2016

HemantMahawar deleted the HM-Patch3 branch January 12, 2017 01:00

HemantMahawar removed the Review - Needed The PR is being reviewed label Jan 30, 2017


		$ProductSemanticVersion = Get-PackageSemanticVersion -Version $PackageVersion

		$PackageVersion = Get-PackageVersionAsMajorMinorBuildRevision -Version $PackageVersion

Search code, repositories, users, issues, pull requests...

Conversation

HemantMahawar commented Nov 22, 2016

Uh oh!

HemantMahawar commented Nov 22, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HemantMahawar Dec 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mirichmo Dec 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

HemantMahawar Dec 3, 2016 •

edited

Loading

mirichmo Dec 3, 2016 •

edited

Loading