Previously, each Angular repository had its own strategy/configuration
for merging pull requests and cherry-picking. We worked out a new
strategy for labeling/branching/versioning that should be the canonical
strategy for all actively maintained projects in the Angular organization.
This PR provides a `ng-dev` merge configuration that implements the
labeling/branching/merging as per the approved proposal.
See the following document for the proposal this commit is based on
for the merge script labeling/branching: https://docs.google.com/document/d/197kVillDwx-RZtSVOBtPb4BBIAw0E9RT3q3v6DZkykU
The merge tool label configuration can be conveniently accesed
within each `.ng-dev` configuration, and can also be extended
if there are special labels on individual projects. This is one
of the reasons why the labels are not directly built into the
merge script. The script should remain unopinionated and flexible.
The configuration is conceptually powerful enough to achieve the
procedures as outlined in the versioning/branching/labeling proposal.
PR Close#38223
The merge tool provides a way for configurations to determine the branches
for a label lazily. This is supported because it allows labels to respect
the currently selected base branch through the Github UI. e.g. if `target: label`
is applied on a PR and the PR is based on the patch branch, then the change
could only go into the selected target branch, while if it would be based on
`master`, the change would be cherry-picked to `master` too. This allows for
convenient back-porting of changes if they did not apply cleanly to the primary
development branch (`master`).
We want to expand this function so that it is possible to report failures if an
invalid target label is appplied (e.g. `target: major` not allowed in
some situations), or if the Github base branch is not valid for the given target
label (e.g. if `target: lts` is used, but it's not based on a LTS branch).
PR Close#38223
The merge script currently accepts a configuration function that will
be invoked _only_ when the `ng-dev merge` command is executed. This
has been done that way because the merge tooling usually relies on
external requests to Git or NPM for constructing the branch configurations.
We do not want to perform these slow external queries on any `ng-dev` command
though, so this became a lazily invoked function.
This commit adds support for these configuration functions to run
asynchronously (by returning a Promise that will be awaited), so that
requests could also be made to the Github API. This is benefical as it
could avoid dependence on the local Git state and the HTTP requests
are more powerful/faster.
Additionally, in order to be able to perform Github API requests
with an authenticated instance, the merge tool will pass through
a `GithubClient` instance that uses the specified `--github-token`
(or from the environment). This ensures that all API requests use
the same `GithubClient` instance and can be authenticated (mitigating
potential rate limits).
PR Close#38223
The merge script uses `git cherry-pick` for both the API merge strategy
and the autosquash strategy. It uses cherry-pick to push commits to
different target branches (e.g. into the `10.0.x` branch).
Those commits never point to the commits that landed in the primary
Github branch though. For the autosquash strategy the pull request number
is always included, so there is a way to go back to the source. On the other
hand though, for commits cherry-picked in the API merge strategy, the
pull request number might not always be included (due to Github's
implementation of the rebase merge method).
e.g.
27f52711c0
For those cases we'd want to link the cherry-picked commits to the
original commits so that the corresponding PR is easier to track
down. This is not needed for the autosquash strategy (as outlined
before), but it would have been good for consistency. Unfortunately
though this would rather complicate the strategy as the autosquash
strategy cherry-picks directly from the PR head, so the SHAs that
are used in the primary branch are not known.
PR Close#37889
We recently added OAuth scope checking to the dev-infra Git client
and started leveraging it for the merge script. We set the `repo` scope
as required for running the merge script. We can loosen this requirement
as in the Angular org where the script is consumed, only pull requests on
public repositories are merged through the script.
This should help with reducing the risk with compromised tokens as no
access had to be granted on `repo:invite`, `repo_deployment` etc.
PR Close#37718
Scripts provided in the `ng-dev` command might use local `git`
commands. For such scripts, we keep track of the branch that
has been checked out before the command has been invoked.
We do this so that we can later (upon command completion)
restore back to the original branch. We do not want to
leave the Git repository in a dirty state.
It looks like this logic currently only deals with branches
but does not work properly when a command is invoked from
a detached head. We can make it work by just checking out
the previous revision (if no branch is checked out).
PR Close#37737
GitClient now uses GithubClient for github API interactions. GithubClient is
a class which extends Octokit and provides a member which allows for GraphQL
requests against the Github GraphQL api, as well as providing convenience methods
for common/repeated Github API requests.
PR Close#37593
Adds support for a caretaker note label to the merge script.
Whenever a configured label is applied, the merge script will
not merge automatically, but instead prompt first in order
to ensure that the caretaker paid attention to the manual
caretaker note on the PR. This helps if a PR needs special
attention.
PR Close#37595
We recently added a better reporting mechanism for oauth tokens
in the dev-infra git util. Unfortunately the logic broke as part
of addressing PR review feedback. Right now, always the empty
promise from `oauthScopes` will be used as `getAuthScopes` considers
it as the already-requested API value. This is not the case as
the default promise is also truthy. We should just fix this by making
the property nullable.
PR Close#37439
Adds an assertion that the provided TOKEN has OAuth scope permissions for `repo`
as this is required for all merge attempts.
On failure, provides detailed error message with remediation steps for the user.
PR Close#37421
Migrate the merge tool in ng-dev to use new logging system rather
than directly calling console.* to create a better experience
for users.
PR Close#37232
The components repo and framework repository follow the same patch
branch concept. We should be able to share a script for determining
these merge branches.
Additonally the logic has been improved compared to the old merge script because
we no longer consult `git ls-remote` unless really needed. Currently,
`git ls-remote` is always consulted, even though not necessarily needed.
This can slow down the merge script and the caretaker process when a
couple of PRs are merged (personally saw around ~4 seconds per merge).
Additionally, the new logic is more strict and will ensure (in most
cases) that no wrong patch/minor branch is determined. Previously,
the script just used the lexicographically greatest patch branch.
This _could_ be wrong when a new patch branch has been created too
early, or by accident.
PR Close#37217
As mentioned in the previous commit, the autosquash strategy has
not been used in the components repo, so we could easily regress.
After thorough manual testing of the autosquash strategy again,
now that the merge script will be moved to framework, it came
to mind that there is a bug with the base revision in the
autosquash merge strategy. The problem is that the base revision
of a given PR is relying on the amount of commits in a PR.
This is prone to error because the amount of commits could easily
change in the autosquash merge strategy, because fixup or squash
commits will be collapsed. Basically invalidating the base revision.
To fix this, we fixate the base revision by determining the actual
SHA. This one is guaranteed to not change after the autosquash rebase.
The current merge script in framework fixates the revision by creating
a separate branch, but there is no benefit in that, compared to just
using an explicit SHA that doesn't need to be cleaned up..
PR Close#37138
The components repo does not use the autosquash merge strategy, so
recent changes to that seem to broke the autosquash strategy.
Since we don't run the rebase in interactive mode, the `--autosquash`
flag has no effect. This is by design in Git. We can make it work by
setting the git sequence editor to `true` so that the rebase seems
like an interactive one to Git, while it isn't one for the user.
This matches conceptually with the merge script currently used in
framework. The only difference is that we allow a real interactive
rebase if the `commit message fixup` label is applied. This allows
commit message modifications (and others) if needed.
PR Close#37138
Integrates the merge script into the `ng-dev` CLI. The goal is that
caretakers can run the same command across repositories to merge a pull
request. The command is as followed: `yarn ng-dev pr merge <number>`.
PR Close#37138
Moves the merge script from the components repository over
to the shared dev-infra package. The merge script has been
orginally built for all Angular repositories, but we just
kept it in the components repo temporarily to test it.
Since everything went well on the components side, we now
move the script over and integrate it into the dev-infra package.
PR Close#37138
Joey Perrott
Paul Gschwendtner