When creating a commit with the git cli, git pre-populates the editor
used to enter the commit message with some comments (i.e. lines starting
with `#`). These comments contain helpful instructions or information
regarding the changes that are part of the commit. As happens with all
commit message comments, they are removed by git and do not end up in
the final commit message.
However, the file that is passed to the `commit-msg` to be validated
still contains these comments. This may affect the outcome of the commit
message validation. In such cases, the author will not realize that the
commit message is not in the desired format until the linting checks
fail on CI (which validates the final commit messages and is not
affected by this issue), usually several minutes later.
Possible ways in which the commit message validation outcome can be
affected:
- The minimum body length check may pass incorrectly, even if there is
no actual body, because the comments are counted as part of the body.
- The maximum line length check may fail incorrectly due to a very long
line in the comments.
This commit fixes the problem by removing comment lines before
validating a commit message.
Fixes#37865
PR Close#38438
Creates a tool within ng-dev to checkout a pending PR from the upstream repository. This automates
an action that many developers on the Angular team need to do periodically in the process of testing
and reviewing incoming PRs.
Example usage:
ng-dev pr checkout <pr-number>
PR Close#38474
Previously commit message types were provided as part of the ng-dev config in the repository
using the ng-dev toolset. This change removes this configuration expectation and instead
predefines the valid types for commit messages.
Additionally, with this new unified set of types requirements around providing a scope have
been put in place. Scopes are either required, optional or forbidden for a given commit
type.
PR Close#38430
We recently updated the benchpress package to have a more loose
Angular core peer dependency, and less other unused dependencies.
We should make sure to use that in the dev-infra package so that
peer dependencies can be satisified in consumer projects, and so
that less unused dependencies are brought into projects.
PR Close#38440
When a commit message fails validation, rather than throwing out the commit message entirely
the commit message is saved into a draft file and restored on the next commit attempt.
PR Close#38304
Adds an ng-dev formatter option to format all of the staged files. This will can
be used to format only the staged files during the pre-commit hook.
PR Close#38402
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
There are a few changes in this PR to ensure conditions that are based
on groups (i.e. `- groups.pending.length == 0`) do not fail the verify
task:
* Remove the warning when a condition is encountered that depends on the
`groups` state. The warning will otherwise be printed once for every
file that triggers the execution of the condition (400,000+ times)
* Add an `unverifiable` flag to `GroupCondition` interface and set it to
true when an error is encountered due to attempting to get the state of
`groups` in a condition
* Ignore any unverifiable conditions when gathering unmatched
conditions. These should not be considered `unmatched` for verification
purposes.
* Print the unverifiable conditions by group in the results
Sample output:
```
┌──────────────────────────────────────────────────────────────────────────────┐
│ PullApprove results by group │
└──────────────────────────────────────────────────────────────────────────────┘
Groups skipped (4 groups)
Matched conditions by Group (37 groups)
Unmatched conditions by Group (0 groups)
Unverifiable conditions by Group (3 groups)
[public-api]
len(groups.pending.exclude("required-minimum-review")...
len(groups.rejected.exclude("required-minimum-review")...
[size-tracking]
len(groups.pending.exclude("required-minimum-review")...
len(groups.rejected.exclude("required-minimum-review")...
[circular-dependencies]
len(groups.pending.exclude("required-minimum-review")...
len(groups.rejected.exclude("required-minimum-review")...
```
PR Close#37798
Conditions can refer to the groups array that is a list of the preceding groups.
This commit adds support to the verification for those conditions.
This commit also adds some tests to the parsing and condition matching
to ensure everything works as expected.
PR Close#38164
* Without this BUILD file we were seeing errors about the reference to
expand_template.bzl in ng_rollup_bundle.bzl because dev-infra/bazel
was not considered a package.
PR Close#38026
Adds Firefox as browser to `dev-infra/browsers` with RBE
compatibility. The default Firefox browser is not compatible similar to
the default Chromium version exposed by `rules_webtesting`.
The Angular Components repository will use this browser target as
it enables RBE support. Also it gives us more flexibility about
the Firefox version we test against. The version provided by
`rules_webtesting` is very old and most likely not frequently
updated (based on past experience).
PR Close#38029
With `rules_nodejs` v2.0.0 being in RC phase currently, we should
make sure that the package is compatible so that we can use it
in the components repo in combination with rules_nodejs v2.0.0.
In v2.0.0 of the NodeJS rules, Bazel workspaces are recommended
to no longer be symlinked under a separate repository. Instead,
bazel rules and targets should be accessed directly from the
user-selected NPM repository. Usually `@npm`, so that the import
changes to `@npm//@angular/dev-infra-private/<..>`.
PR Close#37968
The dev-infra commit message validation optionally can check for lines
to not exceed a given amount of characters. This is desired for most
commit messages, but sometimes not actionable if a long URL is inserted
into the commit message. With this commit, we skip the max line length
check for lines that start with an URL.
PR Close#37890
We recently added a new folder for common bazel utilities
to `dev-infra`. The `ng_rollup_bundle` rule relies on an
utility that is provided by this `bazel/` folder.
Unfortunately though it looks like this folder is currently
not included in the NPM package, so that the `ng_rollup_bundle`
rule does not work as expected. This commit fixes that by
including the bazel utilities in the NPM package.
PR Close#37891
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
This feature will allow us to exclude certain commits from the 100 chars minBodyLength requirement for commit
messages which is hard to satisfy for commits that make trivial changes (e.g. fixing typos in docs or comments).
PR Close#37764
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
* add a param called ng_assets to the component_benchmark macro to allow static assets to be provided to the base angular app, not just through the ts_devserver
PR Close#37695
Refactors the `ng_rollup_bundle` rule to a macro that relies on
the `@bazel/rollup` package. This means that the rule no longer
deals with custom ESM5 flavour output, but rather only builds
prodmode ES2015 output. This matches the common build output
in Angular projects, and optimizations done in CLI where
ES2015 is the default optimization input.
The motiviation for this change is:
* Not duplicating rollup Bazel rules. Instead leveraging the official
rollup rule.
* Not dealing with a third TS output flavor in Bazel.The ESM5 flavour has the
potential of slowing down local development (as it requires compilation replaying)
* Updating the rule to be aligned with current CLI optimizations.
This also _fixes_ a bug that surfaced in the old rollup bundle rule.
Code that is unused, is not removed properly. The new rule fixes this by
setting the `toplevel` flag. This instructs terser to remove unused
definitions at top-level. This matches the optimization applied in CLI
projects. Notably the CLI doesn't need this flag, as code is always
wrapped by Webpack. Hence, the unused code eliding runs by default.
PR Close#37623
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
Cleans up the dependencies used in the shared dev-infra package
configuration. With the recent benchmarking utilities that have
been added, a lot of peer dependencies have been added.
We decided that we don't want to list every used dependencies as
peer dependency as that could result in unnecessary churn/noise
for consumers of the dev-infra package. Additionally, not all parts
of the dev-infra package are necessarily used.
Due to this, we want to apply the following rules for the package
dependencies:
1. If a dependency is only used in a shipped Bazel macro/rule that can be
optionally consumed, omit it from `package.json`. Bazel reports the
missing dependency on its own, so we want to avoid adding it to the
package json file.
2. Otherwise, if the dependency is large and commonly used (like
buildifier), add it to the `peerDependencies`. If not, add it
to the dependencies that are always brought in. We consider it
as acceptable to bring in a few small dependencies that might not
be used or not. Making all of those option would complicate the
use of the dev-infra package.
ds
PR Close#37594
We added a new dependency on `fs-extra` to the dev-infra package. We can
remove this dependency and replace it with `shelljs` that is extensively
used in other places already.
The motiviation is that we can reduce dependencies needed for
for consumption of the shared dev-infra package.
PR Close#37594
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
Adding in a `#` prepended to each PR number in the list of conflicting PRs
found by the discover-new-conflicts script will allow for users to copy
paste the output from the script into a github comment and have the PRs
automatically link.
PR Close#37556
Close#35157
In the current version of zone.js, zone.js uses it's own package format, and it is not following the rule
of Angualr package format(APF), so it is not easily to be consumed by Angular CLI or other bundle tools.
For example, zone.js npm package has two bundles,
1. zone.js/dist/zone.js, this is a `es5` bundle.
2. zone.js/dist/zone-evergreen.js, this is a `es2015` bundle.
And Angular CLI has to add some hard-coding code to handle this case, o5376a8b139/packages/schematics/angular/application/files/src/polyfills.ts.template (L55-L58)
This PR upgrade zone.js npm package format to follow APF rule, https://docs.google.com/document/d/1CZC2rcpxffTDfRDs6p1cfbmKNLA6x5O-NtkJglDaBVs/edit#heading=h.k0mh3o8u5hx
The updated points are:
1. in package.json, update all bundle related properties
```
"main": "./bundles/zone.umd.js",
"module": "./fesm2015/zone.js",
"es2015": "./fesm2015/zone.js",
"fesm2015": "./fesm2015/zone.js",
```
2. re-organize dist folder, for example for `zone.js` bundle, now we have
```
dist/
bundles/
zone.js // this is the es5 bundle
fesm2015/
zone.js // this is the es2015 bundle (in the old version is `zone-evergreen.js`)
```
3. have several sub-packages.
1. `zone-testing`, provide zone-testing bundles include zone.js and testing libraries
2. `zone-node`, provide zone.js implemention for NodeJS
3. `zone-mix`, provide zone.js patches for both Browser and NodeJS
All those sub-packages will have their own `package.json` and the bundle will reference `bundles(es5)` and `fesm2015(es2015)`.
4. keep backward compatibility, still keep the `zone.js/dist` folder, and all bundles will be redirected to `zone.js/bundles` or `zone.js/fesm2015` folders.
PR Close#36540
Looks like we broke the `hasLocalChanges` check in the git client
when we moved it over from the merge script. The problem is that
we are using `git` in the first argument of `git.run`. That
means that we under-the-hood run `git git <..>`.
This commit fixes that, but also switches to a better variant
for ensuring no local changes because it exits with non-zero
when there are local changes.
PR Close#37489
The dev-infra rebase PR script currently does not work due to
the following issues:
1. The push refspec is incorrect. It refers to the `base` of the PR, and
not to the `head` of the PR.
2. The push `--force-with-lease` option does not work in a detached head
as no remote-tracking branch is set up.
PR Close#37489
We recently moved over the git client from the merge script to the
common dev-infra utils. This made specifying a token optional, but
it looks like the logic for sanitizing messages doesn't account
for that, and we currently add `<TOKEN>` between every message
character. e.g.
```
Executing: git <TOKEN>g<TOKEN>i<TOKEN>t<TOKEN>
<TOKEN>s<TOKEN>t<TOKEN>a<TOKEN>t<TOKEN>u<TOKEN>s<TOKEN>
```
PR Close#37489
Historically we have had a pullapprove group `fallback` which acted as
a catch all for files which did not match any other groups. This
group assigned reviews to IgorMinar, however it was not apparent that
this group was assigned. This change removes this assignment. This
group as active should always coincide with failures of the pullapprove
verification script. We continue to have this group as a secondary test
ensuring all files in the repo are captured by the pullapprove config.
PR Close#36456
There is an `exec()` helper provided by `utils/shelljs.ts`, which is a
wrapper around ShellJS' `exec()` with some default options (currently
`silent: true`). The intention is to avoid having to pass these options
to every invocation of the `exec()` function.
This commit updates all code inside `dev-infra/` to use this helper
whenever possible).
NOTE: For simplicity, the `utils/shelljs` helper does not support some
of the less common call signatures of the original `exec()`
helper, so in some cases we still need to use the original.
PR Close#37444
Clean up pullapprove tooling to use newly created common utils.
Additionally, use newly created logging levels rather than
verbose flagging.
PR Close#37338
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
Rename bazel workspace from npm_dev_infra to npm_angular_dev_infra_private to make it clear that this package is private to angular.
Change driver-utilities module_name to match the new bazel workspace name.
Correct a comment by rewording it from "deployed version" to "published version".
Fix merge conflicts in tmpl-package.json
Make "//packages/bazel/src:esm5.bzl" replacement more generalized so that importing from "//packages/bazel" works.
Deleted "dev_infra/*" path from modules/benchmarks tsconfig.
Moved //dev-infra/benchmark/browsers to //dev-infra/browsers.
PR Close#36800
George Kalpakas
Joey Perrott
Paul Gschwendtner
Andrew Scott
Wagner Maciel
Igor Minar
JiaLiPassion
Greg Magolan