Icons should be exempted from Manifest-Metadata-Consistency check

[mdanish-kh]

I'm assuming Icons: are injected somewhere in the publish package step as they are not accepted in the community repository. In that case, Manifest-Metadata-Consistency check should not check against Icons: field. It causes confusion for the PR author as they can't figure out why the bot is reporting a warning since they can't see Icons field in a previous manifest as well.

Some PR examples:

• New version: Google.Chrome.Canary version 131.0.6755.0 #177468
• Microsoft.VisualStudioCode.Insiders_v1.95.0 desktop #177463
• Add version: Mozilla.Thunderbird.lv version 115.10.0 #177421

[mdanish-kh]

[Policy] Issue-Bug
[Policy] Area-Bots

[denelon]

Good catch! Do you have any instances where the PR author said anything in comments?

[denelon]

I'm assuming Icons: are injected somewhere in the publish package step

Yes, we didn't want to create "extra" traffic for publishers or "extra" cost for the data transfer. They are captured during validation so we can store them on the CDN and add the metadata to the manifest based on the "installed" icon.

[mdanish-kh]

Good catch! Do you have any instances where the PR author said anything in comments?

I have not yet. That's probably because all the current example PRs are "automated" PRs, but I imagine a newcomer may feel confused if they contribute to a relevant package. You can filter the existing PRs where this check is applied incorrectly with this GitHub filter. Checks for "in:comments Icons"

https://github.com/microsoft/winget-pkgs/pulls?q=is%3Apr+label%3AManifest-Metadata-Consistency+in%3Acomments+Icons+

[denelon]

OK. The engineering team filed an internal bug to address it.