Skip to content

Found a makeshift fix for the pipeline, to prevent it from mass-spamming "Needs-Author-Feedback" labels.#355564

Open
DandelionSprout wants to merge 1 commit intomicrosoft:masterfrom
DandelionSprout:patch-1
Open

Found a makeshift fix for the pipeline, to prevent it from mass-spamming "Needs-Author-Feedback" labels.#355564
DandelionSprout wants to merge 1 commit intomicrosoft:masterfrom
DandelionSprout:patch-1

Conversation

@DandelionSprout
Copy link
Copy Markdown
Contributor

@DandelionSprout DandelionSprout commented Apr 5, 2026

Checklist for Pull Requests

Manifests

  • Have you checked that there aren't other open pull requests for the same manifest update/change?
  • This PR only modifies one (1) manifest
  • Have you validated your manifest locally with winget validate --manifest <path>?
  • Have you tested your manifest locally with winget install --manifest <path>?
  • Does your manifest conform to the 1.12 schema?

Note: <path> is the directory's name containing the manifest you're submitting.


Notes from me:

  • Summary of what the PR does:
    • No longer assigns the "Needs-Author-Feedback" label.
    • Assigns the "Needs-Attention" label instead.
    • Adds a secondary explanation for what the cause of the Validation-Installation-Error could be.
  • I see no current drawbacks with merging the PR, and in fact it's possibly the most important hotfix the repo has had in years.
Microsoft Reviewers: Open in CodeFlow

@KarbitsCode
Copy link
Copy Markdown
Contributor

No longer assigns the "Needs-Author-Feedback" label.
Assigns the "Needs-Attention" label instead.

Ehm... you could just post a comment and immediately delete that comment to make the bot do that instead.

@DandelionSprout
Copy link
Copy Markdown
Contributor Author

I feel a strong sense of solidarity with newcomers who don't understand why they're told by the pipelines that it's their fault, and I feel so emotionally sorry for them.

@KarbitsCode
Copy link
Copy Markdown
Contributor

KarbitsCode commented Apr 6, 2026

I feel a strong sense of solidarity with newcomers who don't understand why they're told by the pipelines that it's their fault, and I feel so emotionally sorry for them.

Fair enough, that label is very generic anyway and can be caused by anything, sometimes it indeed shouldn't need the PR author feedback when the cause wasn't theirs until confirmed manually by the moderators.

@denelon
Copy link
Copy Markdown
Collaborator

denelon commented Apr 6, 2026

@stphengillie, @Trenly, @mdanish-kh thoughts?

@stephengillie
Copy link
Copy Markdown
Collaborator

Both Needs-Author-Feedback and Needs-Attention have been applied in ways that confuse me for most of my time here. 99% of the "NAF" tickets actually need my attention, and a fair number of "NA" tickets need the author's attention. So my process is to ignore those labels, and @ the author when their attention is needed. Not to sound negative, as I know the current label placement was designed to meet a need, but in a way that isn't clear to me.

Which complicates ignoring those tickets that I've already worked. My automation has a "PR Exclude" list, to prevent it from running PRs in the list. (Over and over again, and responding in the PR, hourly for weeks.)

@denelon
Copy link
Copy Markdown
Collaborator

denelon commented Apr 6, 2026

Originally the label for requesting author feedback was added because many authors didn’t test their PRs before they were submitted - leading to many cases where the dependencies were missing or a driver install message showed up.

Rather than discontinuing the use of that label, I'm leaning towards actionable information an author could follow in the event we run into one of those problems. My intent is not to confuse PR authors, or to increase the level of effort for our moderators to figure out what's wrong when it's something the PR author should be able to fix.

@DandelionSprout
Copy link
Copy Markdown
Contributor Author

Chances are the fix could be modified on very short notice after merging, and in fact even before merging (I've got the "Allow edits by maintainers" button turned on for this PR, which to the best of my knowledge is the default setting).

I've wondered if I should specify -2147467260 as being the wildcard factor in the error's description, or to add a third paragraph to address the separate "No suitable installers" glitches, but my general thought is "Get something initial merged/commited, then the more detailed questions could be asked later".

@denelon
Copy link
Copy Markdown
Collaborator

denelon commented Apr 7, 2026

These labels are applied automatically by the validation pipeline when issues are detected.

Label What the User Can Do
Binary-Validation-Error
Changes-Requested Make the requested changes
Driver-Install Look for switches or a way to pre-install the driverClose the PR as WinGet does not support drivers
Error-Hash-Mismatch Update the manifest with the correct hash
Error-Installer-Availability Update the manifest with a new URLRetry the PR
Highest-Version-Removal Provide a reason for the removal to push it to Needs-Attention
Last-Version-Removal Provide a reason for the removal to push it to Needs-Attention
Manifest-AppsAndFeaturesVersion-Error Update the manifest with the correct ARP entries
Manifest-Installer-Validation-Error Update the manifest with the correct MSIX information
Manifest-Singleton-Deprecated Update the manifest to be a multi-manifest format
Manifest-Validation-Error Update the manifest to comply with the JSON schema
Manifest-Version-Deprecated Update the manifest to a newer schema version
Manifest-Version-Error Update the manifest to a newer schema version
Pull-Request-Error Ensure only one manifestEnsure manifest is under the correct folder
URL-Validation-Error Submit the URL to SmartScreen for validationRe-run the PR at a later date
Validation-404-Error Update the installer URL
Validation-Defender-Error Submit the URL to Defender for validationRe-run the PR at a later date
Validation-Domain Provide evidence of the domain correlation to push the PR to Needs-Attention
Validation-HTTP-Error Update the URLs to use HTTPS
Validation-Indirect-URL Update the URLs to use the direct URLProvide justification for using an indirect URL
Validation-Installation-Error Test the PR locallyReview the pipeline logs for missing dependenciesAsk for clarification / help
Validation-Open-Url-Failed Update the URLs
Validation-Shell-Execute Test the PR locallyReview the pipeline logs for missing dependenciesAsk for clarification / help
Validation-Unattended-Failed Test the PR locallyReview the pipeline logs for missing dependenciesAsk for clarification / help

These labels are applied by repository maintainers when an issue cannot be automatically validated or requires policy decisions.

Label What the User Can Do Notes
Interactive-Only-Installer Close the PR as the installer does not support silent install at all Manual
License-Blocks-Install Add the agreements section Manual
Manifest-Content-Incomplete Update the manifest with sufficient information Manual
Network-Blocker Contact the ISV to whitelist Azure VM IPsClose the PR as nothing can be done Manual
Portable-Archive Close the PR until support is added into the client Manual
Portable-Jar Close the PR until support is added into the client Manual
Scripted-Application Close the PR until policy changes Manual
Validation-Error Test the PR locally and ensure that it can install correctly Manual
Validation-Merge-Conflict Resolve the merge conflict Manual
Validation-Missing-Dependency Add the dependency Manual
WindowsFeatures Add the dependency on the Windows Feature Manual / Appears unused
Zip-Binary Update the manifest to have ArchiveBinariesDependOnPath Manual
When modifying validation-pipeline.yaml without repo permissions Remove the file from their PR Manual
No-Recent-Activity Respond to the PR Manual

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Author-Not-Authorized Needs-Attention This work item needs to be reviewed by a member of the core team. Project-File

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants