Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[New Feature]: wingetbot should ask the owner of the package in Auth.csv #152097

Open
CoolPlayLin opened this issue May 3, 2024 · 2 comments
Open
Labels
Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work.

Comments

@CoolPlayLin
Copy link
Contributor

Description of the new feature/enhancement

I have noticed that all of updating Pull Requests of packages which require approval from their owners in Auth.csv need to be added Needs-Review This work item needs to be reviewed by a member of another team. by @stephengillie and ask owners as well.It is unreasonable because it slows down the speed of updating packages.

I think validation pipeline should add a step which let wingetbot ask owners automatically when it passes all the validation and add Needs-Review This work item needs to be reviewed by a member of another team. as well.

Proposed technical implementation details (optional)

No response

@CoolPlayLin CoolPlayLin added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label May 3, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Triage This work item needs to be triaged by a member of the core team. label May 3, 2024
@Trenly
Copy link
Contributor

Trenly commented May 3, 2024

I think one issue here is that auth.csv is not integrated at all with the validation pipelines. It is only used by @stephengillie's automation as a temporary thing until the official support for verified publishers is implemented

@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Triage This work item needs to be triaged by a member of the core team. label May 3, 2024
@stephengillie
Copy link
Collaborator

This is a good idea. It goes along with my goal of moving checks forward in the PR process.

A PR's journey through the WinGet pipeline system:

  • PR committed - manually or with a tool.
  • PR pulled into Automatic Validation Pipeline.
    • On fail, add label.
      • Pipeline errors - these often have the label Internal-Error or Internal-Error-Dynamic-Scan. The former are usually ephemeral and disappear on retry. The latter are sometimes ephemeral, and sometimes happen to a package on every run - it's far enough along that the package can be manually validated. Manual validation might still fail on one of the following errors, and has to manually update labels and comment in the PR similar to how the pipeline would.
      • PR and Manifest errors - these can often be remediated in PR, then retry and pass.
      • Defender and scan errors - some can linger in the fail-and-remediation state ("Defender Loop") for an extended duration before passing.
      • Installer and application errors - these can sometimes be remediated in PR, by adding data such as switches or dependencies. If so, retry and pass.
      • Legal and political issues - these can be hard blockers. Some might need the manifest schema to be updated with additional fields, and those fields populated with legal agreements, before the PR can pass.
      • Unfortunately, for some PRs, the next step is closure. The path to this step isn't always straightforward, and some linger here for an extended duration as well. Closed PRs can be reopened for a good reason. Feel free to ask.
    • On pass, continue.
  • SDL checks occur at some point. These don't take very long, unless you're waiting on the PR.
  • PR pends for community and moderator review. (Review "pool")
    • On fail, add comment.
      • Installer: Duplicate PRs, version mismatches between manifest and registry, different installer types.
      • Locale: Incorrect PackageName, ReleaseNotes not in locale's natural language, PackageUrl not leading to InstallerUrl, and other errata.
    • On pass, continue.
  • PR pends for moderator approval. (Approval pipeline)
    • On fail, add comment.
      • Auth fail - Package has Auth strictness of "must" and submitter isn't on the list. Ask someone who IS on the list for approval.
      • Version parameter fail - the number of version parameters (data between dots, such as major and minor version numbers) has changed. This is common for some developers, and an exception list is currently manually implemented.
      • Version number contains spaces fail - this check needs to be reimplemented. It was meant to catch an automation bug adding spaces after the dots in PackageVersion numbers.
      • Review fail - PackageIdentifier has review notes blocking approval. Post them.
      • Agreements fail - PackageIdentifier has EULA but PR is missing the AgreementsUrl. Post this.
      • Words filter fail - Manifest contains words (such as "EULA") that are restricted, because they might indicate another check has failed or been skipped. Post about these.
      • AnF fail - missing the "AppsAndFeaturesEntries" entry but present in previous PR. This check needs to be updated. Usually only block on DisplayVersion, but also note if there are more than 3 of these missing.
      • InstallerUrl contains PackageVersion - Doesn't block but is informative. Should be rebuilt to include a vanity URL detector, and also detect if the InstallerUrl shows previous version.
      • Files removed - if the PR has more than 2 files, and it's not a removal, check if the previous version had at least as many files. To prevent a PR from leaving out localization files from the previous version.
      • OR Last Version Remaining fail - If it's a removal, check if it's the highest version. If it is, ask if it's available from another location.
    • On pass, approve.
  • PR pends for publish pipeline.
    • Publish converts repo to an XML database and compresses into MSIX.
    • Uploads to storage location, refreshes CDN.
  • Package is available to users.
    (Goal is to make this have 1 remediation loop instead of 3.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work.
Projects
None yet
Development

No branches or pull requests

3 participants