Add support for markets #1806
Add support for markets #1806
Conversation
ghost
added
Area-Manifest
| if (!installer.Markets.AllowedMarkets.empty()) | ||
| { | ||
| // Inapplicable if NOT found | ||
| if (installer.Markets.AllowedMarkets.end() == std::find(installer.Markets.AllowedMarkets.begin(), installer.Markets.AllowedMarkets.end(), m_market)) |
There was a problem hiding this comment.
Are these market strings normalized to allow for this direct equality comparison? Most likely not, and we need to either normalize them or perform a normalizing comparison. Either way, I would suggest a helper type to make it easier.
There was a problem hiding this comment.
I made the comparison case insensitive, but after re-reading your comment I'm not sure if that was what you meant. The manifest schema requires the markets to be two uppercase letters, so we shouldn't need any normalization there, although I'm not sure if the region from the OS could not be uppercase.
There was a problem hiding this comment.
Unless this is a performance critical path and we find this to be an issue, I would leave it as case insensitive (also unless it turns out that case sensitivity is actual part of the definition of markets). We don't own the validation on both sides, so better to be safe.
ghost
added
the
Needs-Author-Feedback
Co-authored-by: JohnMcPMS <[email protected]>
ghost
removed
the
Needs-Author-Feedback
| strstr << ", "; | ||
| } | ||
|
|
||
| strstr << toString(item); |
There was a problem hiding this comment.
Which type(s) were not being output properly by the <<? I would expect any stringish type we use to work without a coercion function.
Not critical, but I like to understand 😄
There was a problem hiding this comment.
Utility::Architecture for the applicable architectures filter. It was coming out as an integer.
Adding support for filtering installers based on markets in the manifest.
ManifestComparator. Filter uses the current market as returned byGetUserDefaultGeoName().Closes #1241
Microsoft Reviewers: Open in CodeFlow