Additional package metadata #39

danbelcher-MSFT · 2017-03-14T23:13:11Z

This change adds additional package metadata to the output .JSON from the New-SubmissionPackage command.

New metadata is added to each object in 'applicationPackages'. The new fields are:
-version
-architecture
-targetPlatform
-languages
-capabilities
-targetDeviceFamilies
-targetDeviceFamiliesEx
-minOSVersion
-innerPackages

These properties are removed during a submission because they are re-calculated by the Store.

Resolves #16

msftclas · 2017-03-14T23:13:15Z

@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

Overall, I really like this change. I like the way that you abstracted out the logic. Just looks like some minor changes to make before we bring it in.

HowardWolosky · 2017-03-16T19:19:18Z

StoreBroker/Helpers.ps1

@@ -813,3 +813,52 @@ function Resolve-UnverifiedPath
        return $resolvedPath.ProviderPath
    }
 }
+
+function Remove-UnofficialSubmissionProperties


With the exception of Initialize-HelpersGlobalVariables due to some of those globals being used within Helpers.ps1 itself, Helpers is really designed to be "commands that PowerShell doesn't natively have, but would be useful." This method should probably be moved into StoreIngestionApi.psm1 for sharing.

Moved to StoreIngestionApi.psm1

HowardWolosky · 2017-03-16T19:21:52Z

StoreBroker/StoreIngestionFlightingApi.ps1

-    # Now, we'll remove the appId property since it's not really valid in submission content.
-    # We can safely call this method without validating that the property actually exists.
-    $submission.PSObject.Properties.Remove('appId')
+    # Remove properties that aren't valid in submission content.


Due to the very well named helper function here, this comment adds no value.

HowardWolosky · 2017-03-16T19:21:56Z

StoreBroker/StoreIngestionIapApi.ps1

-    # Now, we'll remove the iapId property since it's not really valid in submission content.
-    # We can safely call this method without validating that the property actually exists.
-    $submission.PSObject.Properties.Remove('iapId')
+    # Remove properties that aren't valid in submission content.


Due to the very well named helper function here, this comment adds no value.

HowardWolosky · 2017-03-16T19:25:04Z

StoreBroker/PackageTool.ps1

+)
+
+# String constants for UAP namespaces
+$script:uapNamespaces = @(


It seems like this approach puts StoreBroker in a maintenance scenario where we'd have to keep the list up-to-date each time the Windows SDK introduces a new namespace. All of these namespaces have "Windows10" in them...can we trigger off that instead? Seems like a more future-proof approach.

I'm fine with making this change, however, the reason we asked Store team for the set of namespaces to use was because you didn't want to do a generic search for "Windows10". Still prefer the search over the namespace set?

Will change to the string search. Agreed that it is more future-proof.

Agreed to use the string search instead of maintaining the namespace set.

HowardWolosky · 2017-03-16T19:31:23Z

StoreBroker/PackageTool.ps1

@@ -24,6 +24,36 @@ $script:s_OutPath = "OutPath"
 $script:s_OutName = "OutName"
 $script:s_DisableAutoPackageNameFormatting = "DisableAutoPackageNameFormatting"

+# String constants for application metadata
+$script:applicationMetadataProperties = @(


I think i'd rather see this contained as a local var within New-ApplicationMetadataTable

Discussed in a later comment. These properties need to be referenced elsewhere so it's better to keep as a script var.

HowardWolosky · 2017-03-16T20:09:23Z

StoreBroker/PackageTool.ps1

+
+        # Grab an arbitrary one and use that version.
+        $arch = $Metadata.innerPackages.Keys | Sort-Object | Select -First 1
+        $version = $Metadata.innerPackages.$arch.version


I was thinking maybe we should change to using the bundle version in this scenario. Can you confirm with Release that they would be ok with that?

I asked Release team and they said they do not want that. They want to always use the appx version, acknowledging that inner appx packages might not necessarily have the same version. We will simply use the version of the last appx processed.

Alrighty. Sounds good. Just wanted to make sure we checked with them first.

HowardWolosky · 2017-03-16T20:13:12Z

StoreBroker/PackageTool.ps1

+    $deviceFamilyCollection = @()
+    foreach ($family in $Metadata.targetDeviceFamiliesEx)
+    {
+        $index = $family.IndexOf($target)


I think this section could be simplified significantly to just do this:
$target -replace '^Windows.', ''
(that says, if the string starts with "Windows.", replace that with nothing. Then return the resulting string regardless.

Great tip, thanks!

HowardWolosky · 2017-03-16T20:15:11Z

StoreBroker/PackageTool.ps1

+    $formattedBundleTags = @($Metadata.name, $version, $architectureTag)
+    if ($deviceFamilyCollection.Count -gt 0)
+    {
+        $formattedBundleTags = @(($deviceFamilyCollection | Sort-Object) -join '.') + $formattedBundleTags


I feel like this line would read easier as:
$formattedBundleTags += ($deviceFamilyCollection | Sort-Object) -join '.')

Hmm, the code you suggested would add the deviceFamilyCollection to the end of the name, whereas my code is explicitly adding it as the beginning. The current implementation follows the structure requested by Release team. Should I ask them if they'd be okay with this change? Or would you rather keep as is?

Keep as is. You're totally right. Misread the code.

Agreed to keep as is.

HowardWolosky · 2017-03-16T20:17:07Z

StoreBroker/PackageTool.ps1

            }
        }
+
+        $metadata.name = Get-FormattedFilename -Metadata $metadata


I'm confused that the inner methods are calculating $metadata.name, but then you're replacing it here. Should they be storing their property somewhere else? Maybe use a different property name here like formattedFileName?

I see the confusion. I agree with the suggestion to change this property to 'formattedFileName'.

HowardWolosky · 2017-03-16T20:20:48Z

StoreBroker/Helpers.ps1

@@ -813,3 +813,52 @@ function Resolve-UnverifiedPath
        return $resolvedPath.ProviderPath
    }
 }
+
+function Remove-UnofficialSubmissionProperties


Nice abstraction

HowardWolosky · 2017-03-23T20:40:32Z

Do a rebase to flatten your change out to a single change, and update the version in the psd1 (increment the minor version (major.minor.patch) and reset patch to 0).

… the New-SubmissionPackage command. New metadata is added to each object in 'applicationPackages'. The new fields are: -version -architecture -targetPlatform -languages -capabilities -targetDeviceFamilies -targetDeviceFamiliesEx -minOSVersion -innerPackages These properties are removed during a submission because they are re-calculated by the Store. Resolves #16

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

danbelcher-MSFT requested a review from HowardWolosky March 16, 2017 19:09

HowardWolosky requested changes Mar 16, 2017

View reviewed changes

HowardWolosky approved these changes Mar 23, 2017

View reviewed changes

danbelcher-MSFT merged commit 391954b into microsoft:master Mar 23, 2017

danbelcher-MSFT deleted the forked/AdditionalPackageMetadata branch January 30, 2018 01:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Additional package metadata #39

Additional package metadata #39

danbelcher-MSFT commented Mar 14, 2017

msftclas commented Mar 14, 2017

HowardWolosky left a comment

HowardWolosky Mar 16, 2017

danbelcher-MSFT Mar 16, 2017

HowardWolosky Mar 16, 2017

HowardWolosky Mar 16, 2017

HowardWolosky Mar 16, 2017

danbelcher-MSFT Mar 16, 2017

danbelcher-MSFT Mar 16, 2017

danbelcher-MSFT Mar 16, 2017

HowardWolosky Mar 16, 2017

danbelcher-MSFT Mar 16, 2017

HowardWolosky Mar 16, 2017

danbelcher-MSFT Mar 16, 2017

HowardWolosky Mar 16, 2017

HowardWolosky Mar 16, 2017

danbelcher-MSFT Mar 16, 2017

HowardWolosky Mar 16, 2017

danbelcher-MSFT Mar 16, 2017

HowardWolosky Mar 16, 2017

danbelcher-MSFT Mar 16, 2017

HowardWolosky Mar 16, 2017

danbelcher-MSFT Mar 16, 2017

HowardWolosky Mar 16, 2017

HowardWolosky commented Mar 23, 2017

Additional package metadata #39

Additional package metadata #39

Conversation

danbelcher-MSFT commented Mar 14, 2017

msftclas commented Mar 14, 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

HowardWolosky commented Mar 23, 2017