Azure / azure-rest-api-specs

The source for REST API specifications for Microsoft Azure.
MIT License
2.6k stars 5.01k forks source link

Force refresh of TypeSpec PRs when version is updated in main #28179

Open mikeharder opened 6 months ago

mikeharder commented 6 months ago

PR #28138 caused a break in main. The PR was started when main was using TypeSpec 0.53, and check TypeSpec Validation passed on this version. When the PR was merged, main had upgraded to TypeSpec 0.54, but check TypeSpec Validation was not re-run. So the PR was able to merge with all checks green, but then cause a failure in main.

One way to fix this would be to enable setting "Require branches to be up to date before merging" in the branch protection for main, but we feel this is infeasible due to the large number of unrelated PRs submitted to this repo.

Another option might be to have a pipeline that manually triggers a refresh of all open PRs (or just TSP PRs) when TSP is upgraded in main. It could use a GitHub API to find all PRs and re-trigger checks.

return github.pulls.updateBranch({
  owner: context.repo.owner,
  repo: context.repo.repo,
  pull_number: pr.number,
});

The manual refresh could work two ways:

  1. Force re-run of checks, but don't merge to PR branch
  2. Force update of PR branch from main

The former is more lightweight, since it doesn't push any changes to PRs we didn't create. The latter might help spec authors working locally, since they generally forget to merge from main when TSP is upgraded.

weshaggard commented 6 months ago

As I mentioned in the Teams chat I don't think we should do anything here but definitely don't like to the idea of automatically updating branches. We should stop having breaking changes in our tools, but in exceptional cases we can rerun just the TypeSpec validation pipeline or just deal with the breaks in main.

Another idea is we could also consider enabling merge queue if we really worry about having main broken temporarily, but I don't think it is all that big of a deal if it is broken for a little while.

mikeharder commented 6 months ago

We should stop having breaking changes in our tools

I don't think this is a realistic goal. For example, we want to be able to keep adding more rules to both the TypeSpec Linter and TypeSpecValidation. We rely on the ability to atomically add the new rules and fix exsting specs. The only problem are the in-flight PRs that don't rerun checks (or merge from main) before merging to main.