The `ng-dev release publish` command needs to check whether a commit
closed a pull request. This is implemented via checking the commit
message for specific closing keywords referencing the pull request
number.
The regex used previously failed to correctly ensure that the specified
pull request was referenced. For example, it would allow `#12345` to
also match for `#1234`.
This commit fixes the regex.
PR Close#39135
Bump Chrome to the next stable release (84.0.4147) by following the
instructions in dev-infra/browsers/README.md.
With Chrome 86 about to be released as stable, the current local version
(Chrome 83) is starting to lag behind. It also contains a bug that
blocks Angular unit and integration tests from using Trusted Types.
PR Close#39179
Updates to rules_nodejs 2.2.0. This is the first major release in 7 months and includes a number of features as well
as breaking changes.
Release notes: https://github.com/bazelbuild/rules_nodejs/releases/tag/2.0.0
Features of note for angular/angular:
* stdout/stderr/exit code capture; this could be potentially be useful
* TypeScript (ts_project); a simpler tsc rule that ts_library that can be used in the repo where ts_library is too
heavy weight
Breaking changes of note for angular/angular:
* loading custom rules from npm packages: `ts_library` is no longer loaded from `@npm_bazel_typescript//:index.bzl`
(which no longer exists) but is now loaded from `@npm//@bazel/typescript:index.bzl`
* with the loading changes above, `load("@npm//:install_bazel_dependencies.bzl", "install_bazel_dependencies")` is
no longer needed in the WORKSPACE which also means that yarn_install does not need to run unless building/testing
a target that depends on @npm. In angular/angular this is a minor improvement as almost everything depends on @npm.
* @angular/bazel package is also updated in this PR to support the new load location; Angular + Bazel users that
require it for ng_package (ng_module is no longer needed in OSS with Angular 10) will need to load from
`@npm//@angular/bazel:index.bzl`. I investigated if it was possible to maintain backward compatability for the old
load location `@npm_angular_bazel` but it is not since the package itself needs to be updated to load from
`@npm//@bazel/typescript:index.bzl` instead of `@npm_bazel_typescript//:index.bzl` as it depends on ts_library
internals for ng_module.
* runfiles.resolve will now throw instead of returning undefined to match behavior of node require
Other changes in angular/angular:
* integration/bazel has been updated to use both ng_module and ts_libary with use_angular_plugin=true.
The latter is the recommended way for rules_nodejs users to compile Angular 10 with Ivy. Bazel + Angular ViewEngine is
supported with @angular/bazel <= 9.0.5 and Angular <= 8. There is still Angular ViewEngine example on rules_nodejs
https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular_view_engine on these older versions but users
that want to update to Angular 10 and are on Bazel must switch to Ivy and at that point ts_library with
use_angular_plugin=true is more performant that ng_module. Angular example in rules_nodejs is configured this way
as well: https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular. As an aside, we also have an
example of building Angular 10 with architect() rule directly instead of using ts_library with angular plugin:
https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular_bazel_architect.
NB: ng_module is still required for angular/angular repository as it still builds ViewEngine & @angular/bazel
also provides the ng_package rule. ng_module can be removed in the future if ViewEngine is no longer needed in
angular repo.
* JSModuleInfo provider added to ng_module. this is for forward compat for future rules_nodejs versions.
PR Close#39182
Updates to rules_nodejs 2.2.0. This is the first major release in 7 months and includes a number of features as well
as breaking changes.
Release notes: https://github.com/bazelbuild/rules_nodejs/releases/tag/2.0.0
Features of note for angular/angular:
* stdout/stderr/exit code capture; this could be potentially be useful
* TypeScript (ts_project); a simpler tsc rule that ts_library that can be used in the repo where ts_library is too
heavy weight
Breaking changes of note for angular/angular:
* loading custom rules from npm packages: `ts_library` is no longer loaded from `@npm_bazel_typescript//:index.bzl`
(which no longer exists) but is now loaded from `@npm//@bazel/typescript:index.bzl`
* with the loading changes above, `load("@npm//:install_bazel_dependencies.bzl", "install_bazel_dependencies")` is
no longer needed in the WORKSPACE which also means that yarn_install does not need to run unless building/testing
a target that depends on @npm. In angular/angular this is a minor improvement as almost everything depends on @npm.
* @angular/bazel package is also updated in this PR to support the new load location; Angular + Bazel users that
require it for ng_package (ng_module is no longer needed in OSS with Angular 10) will need to load from
`@npm//@angular/bazel:index.bzl`. I investigated if it was possible to maintain backward compatability for the old
load location `@npm_angular_bazel` but it is not since the package itself needs to be updated to load from
`@npm//@bazel/typescript:index.bzl` instead of `@npm_bazel_typescript//:index.bzl` as it depends on ts_library
internals for ng_module.
* runfiles.resolve will now throw instead of returning undefined to match behavior of node require
Other changes in angular/angular:
* integration/bazel has been updated to use both ng_module and ts_libary with use_angular_plugin=true.
The latter is the recommended way for rules_nodejs users to compile Angular 10 with Ivy. Bazel + Angular ViewEngine is
supported with @angular/bazel <= 9.0.5 and Angular <= 8. There is still Angular ViewEngine example on rules_nodejs
https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular_view_engine on these older versions but users
that want to update to Angular 10 and are on Bazel must switch to Ivy and at that point ts_library with
use_angular_plugin=true is more performant that ng_module. Angular example in rules_nodejs is configured this way
as well: https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular. As an aside, we also have an
example of building Angular 10 with architect() rule directly instead of using ts_library with angular plugin:
https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular_bazel_architect.
NB: ng_module is still required for angular/angular repository as it still builds ViewEngine & @angular/bazel
also provides the ng_package rule. ng_module can be removed in the future if ViewEngine is no longer needed in
angular repo.
* JSModuleInfo provider added to ng_module. this is for forward compat for future rules_nodejs versions.
@josephperrott, this touches `packages/bazel/src/external.bzl` which will make the sync to g3 non-trivial.
PR Close#37727
This commit adds a new command to the `ng-dev` suite, which verifies that the NgBot YAML config is
correct. It also adds this command to the `lint` CircleCI job so that we execute this check while
running CI.
This should help prevent syntax errors similar to the one introduced in:
393ce5574b
PR Close#39071
Creates a tool for staging and publishing releases as per the
new branching and versioning that has been outlined in the following
document. The tool is intended to be used across the organization to
ensure consistent branching/versioning and labeling:
https://docs.google.com/document/d/197kVillDwx-RZtSVOBtPb4BBIAw0E9RT3q3v6DZkykU/edit#heading=h.s3qlps8f4zq7dd
The tool implements the actions as outlined in the following
initial plan: https://hackmd.io/2Le8leq0S6G_R5VEVTNK9A.
The implementation slightly diverged in so far that it performs
staging and publishing together so that releasing is a single
convenient command. In case of errors for which re-running the
full command is not sufficient, we want to consider adding
recover functionality. e.g. when the staging completed, but the
actual NPM publishing aborted unexpectedly due to build errors.
PR Close#38656
Introduces a new command for `ng-dev release`, so that the NPM
dist tag can be set for all configured NPM packages. This command
can be useful in case a manual tag needs to be set, but it is
primarily used by the release tooling when a new stable version
is cut, and when the previous patch branch needs to be set as LTS
version through a `v{major}-lts` dist tag.
It is necessary to have this as a command so that the release tool
can execute it for old branches where other packages might have been
configured. This is similar to the separate `ng-dev build` command
that we created.
Note that we also added logic for spawning a process conveniently
with different "console output" modes. This will be useful for
other command invocations in the release tool and it's generally
better than directly using native `child_process` as that one doesn't
log to the dev-infra debug log file.
PR Close#38656
Adds a command for building all release packages. This command
is primarily used by the release tool for building release output
in version branches. The release tool cannot build the release packages
configured in `master` as those packages could differ from the
packages available in a given version branch. Also, the build process
could have changed, so we want to have an API for building
release packages that is guaranteed to be consistent across branches.
PR Close#38656
Moves the existing `ng-dev release env-stamp` command into a
subfolder so that the staging/publish tool can have its own
dedicated folder (without being polluted by the env-stamp logic).
Every subcommand should be in its own folder.
PR Close#38656
Adds a new folder to dev-infra where shared testing utilities
could be placed in. This commit already adds initial testing
utilities for dealing with the `GitClient` and SemVer versions.
The `GitClient` in the testing utilities simulates actual Git
behavior in a virtual manner. It's not complete at all, but can
be extended based on our needs. The currently implemented commands
are the most basic ones that we'd need for our release tooling.
PR Close#38656
Adds a method for printing active release trains for a configured
project. This is helpful for the release tool that will print
the active release trains. Also this can be useful for the
caretaker status command, where we could print the active
version branches (i.e. "is there currently a feature-freeze branch").
PR Close#38656
Adds logic for determining active LTS branches for a given
release configuration. The active LTS branches can be determined
by querying NPM and matching dist tags against a specific
pattern. i.e. `v{major}-lts`.
This logic will be useful for the release tool that supports
publishing of active LTS version branches.
PR Close#38656
Cleans up outdated comments in the shared dev-infra Git
utilities. We also export the Graphql client for consistency
as we expose the `GithubClient` and `GitClient` too.
PR Close#38656
We initially added logic for determining active release trains into
the merge script. Given we now build more tools that rely on this
information, we move the logic into a more general "versioning" folder
that can contain common logic following the versioning document for the
Angular organization.
PR Close#38656
Introduces a new configuration for the `ng-dev release` command. This
configuration will be the source of truth for all release packages
and how they can be built.
Additionally, in a temporary manner where each project has its own
way of generating the changelog, the changelog generation can be
configured. This will be removed in the future when there is
canonical changelog generation in the dev-infra shared package.
PR Close#38656
Exposes logic for dealing with LTS branches, so that the release
tool can re-use it for cutting LTS patch releases.
Eventually, we can move all of this logic to a more dedicated
folder instead of having it inside the merge folder.
PR Close#38656
Instead of maintaining multiple interface for grouping
owner name and repo name, we expose a shared interface
describing a Github repository.
One unfortunate downside is that the GraphQL Github
and Rest API diverge slightly with the key for the
repository name. i.e. rest uses `repo` for the name
of a repository, while GraphQL uses `name` for the name.
If that would be consistent, we could use the rest operator
to pass a repository to the Octokit REST or GraphQL API. This
does not work, so we have a small manual overhead as seen
in the `branches.ts` file.
PR Close#38656
The dev-infra package is currently built with Bazel and ts-node.
In Bazel, the shared tsconfig from the `packages/` folder is used.
This means that the code is built in strict mode, but IDEs and
ts-node do not know about the strictness. This is because the tsconfig
is part of the `packages` folder and not accessible from the
dev-infra package. We fix this by adding an IDE and ts-node specific
tsconfig to the dev-infra package.
This helps with spotting compilation failures before building
with Bazel / waiting for CI to check build state.
PR Close#38656
Previously, the logic for determing the active release trains did not
return the resolved version of a release train. With the publish script
being created, we need this information and can just pass it through,
so that we do not need to fetch and parse the package.json of given
branches multiple times.
PR Close#38656
Sets up the NPM `ora` package in the project and in dev-infra,
so that we can show progress spinners when needed. This is useful
in the publish release script when we wait for a pull request to
be merged.
PR Close#38656
The git client respects the `SpawnSyncOptions` when a command
is executed. Currently it does not hide the command info
messages when commands are run in silent mode.
We fix this as part of this commit, so that the command info
is only printed to `debug` if `stdio` is set to `ignore`.
Additonally, the github token is made public so that it can be
used by commands if other repositories like forks are targeted.
PR Close#38656
Instead of repeating the logic for adding the github token to
a repository git url, we add a shared function for automatically
computing the URls with token.
Additionally, URLs for updating/generating tokens have been moved
to a dedicated file in the `utils` folder. Also while being at it,
the yargs github token helper is also moved into the dedicated
Git/Github related util folder.
PR Close#38656
This commit adds a logic to ouput the number of new and fixed cycles after running circular
dependency checker. This information is useful to better understand an impact of changes in case
the number of new/fixed cycles is relatively big.
PR Close#38805
During the merge process, all validations have already been completed so git commit
hooks can be safely skipped. This additionally, prevents errors from occuring which
would be caused the commit hooks executing, such as when yarn updates and then yarn
commands are unable to run within the same process.
PR Close#38888
As part of the commit message conformance check, local commit message checks are
made to be warnings rather than failures. An additional local option is also in
place to allow for the commit message validation failures to be considered errors
instead.
PR Close#38784
Currently we validate the configuration file on each `getConfig`
invocation. We can only validate once since the configuration
is cached.
Also while being at it, renames the cache variables to lower-case as those
do not represent constants (which are convention-wise upper case).
PR Close#38808
As not all users, particularly contributors consistently contributing with a deep
understanding of our commit message guidelines, will not want to rely on the
commit message wizard, we allow a user to opt out of using this wizard during
commit message creation.
PR Close#38701
Create a utility for loading a local user configuration object to describe
local configuration values, such as skipping the commit message wizard.
PR Close#38701
Previously, the validateCommitMessage function ran validation and logged the results.
The validateCommitMessage function now returns an object containing the validation
results and the cli action functions are instead responsible for logging the results.
This is being done as a prefactor for a change which allows for commit message
hook validation to be either a blocking error or a warning.
PR Close#38703
The angular team relies on a number of services for hosting code, running CI, etc. This
tool allows for checking the operational status of all services at once as well as the current
state of the repository with respect to merge and triage ready issues and prs.
PR Close#38601
Creates a mixin for requiring a github token to be provided to a command. This mixin
allows for a centralized management of the requirement and handling of the github-token.
PR Close#38630
Currently the merge script default branch configuration throws an error
if an unexpected version branch is discovered. The error right now
assumes to much knowledge of the logic and the document outlining
the release trains conceptually.
We change it to something more easy to understand that doesn't require
full understanding of the versioning/labeling/branching document that
has been created for the Angular organization.
PR Close#38622
Creates infrastructure to write outputs of command runs to ng-dev log file.
Additionally, on commands which fail with an exit code greater than 1, an
error log file is created with the posix timestamp of the commands run time
as an identifier.
PR Close#38599
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
Migrate to using .ng-dev directory for ng-dev configuration to better
allow management of the configuration using multiple files. The
intention is to prevent the config file from becoming unruly.
PR Close#37142
Migrate the release tool in ng-dev to use new logging system rather
than directly calling console.* to create a better experience
for users.
PR Close#37232
Migrate the ts-circular-dependencies tool in ng-dev to use new logging system rather
than directly calling console.* to create a better experience
for users.
PR Close#37232
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
Migrate the pullapprove tool in ng-dev to use new logging system rather
than directly calling console.* to create a better experience
for users.
PR Close#37232
Migrate the rebase tool in ng-dev to use new logging system rather
than directly calling console.* to create a better experience
for users.
PR Close#37232
Migrate the discover-new-conflicts tool in ng-dev to use new logging system
rather than directly calling console.* to create a better experience
for users.
PR Close#37232
Migrate the commit-message tool in ng-dev to use new logging system rather
than directly calling console.* to create a better experience
for users.
PR Close#37232
Migrate the formatting tool in ng-dev to use new logging system rather
than directly calling console.* to create a better experience
for users.
PR Close#37232
Adds .group and .groupEnd functions to each of the logging functions
to allow creating groups in the logged output. Additionally removes
the color parameter from logging functions, in favor of the color
being applied to the string at the call site.
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
We recently added support for automatic registration of `ts-node`
when the dev-infra configuration is loaded.
In addition to registering ts-node, we should also ensure that the
`commonjs` module is set up. By default, `ts-node` would use ES module
imports that are not supported by default in NodeJS.
PR Close#37217
Creates common logging functions at different levels. Allows for providing
logging statements which are actually printed to the console based on the
LOG_LEVEL environment variable.
PR Close#37192
`ts-node` is now an optional peer dependency of the shared dev-infra
package. Whenever a `ng-dev` command runs, and a TypeScript-based
configuration file exists, `ts-node` is set up if available.
That allows consumers of the package (as the components repo) to more
conveniently use a TypeScript-based configuration for dev-infra.
Currently, commands would need to be proxied through `ts-node`
which rather complicates the setup:
```
NG_DEV_COMMAND="ts-node ./node_modules/@angular/dev-infra-private/cli.js"
```
I'm thinking that it should be best-practice to use TypeScript for
writing the configuration files. Given that the tool is used primarily
in Angular projects (for which most sources are TypeScript), this should
be acceptable.
PR Close#37196
Creates a tool in ng-dev which rebases a PR automatically and pushes
the rebase commits back to the PR. This is meant to be a replacement
to the local merge script currently in the repo and currently has
feature parity.
PR Close#37055
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
Creates a tool in ng-dev to determine the PRs which become conflicted
by merging a specified PR. Often the question is brought up of how
many PRs require a rebase as a result of a change. This script allows
to determine this impact.
PR Close#37051
Adds the gitub configuration to the ng-dev configuration. This github
configuration provides information needed for making API requests to
github. Upcoming tooling related PRs will require these API requests
being possible.
PR Close#37097
Adds a gitub object to the common configuration for ng-dev. This github
configuration provides information needed for making API requests to
github.
PR Close#37097
Release commits do not require a commit body as the context, usually
provided in commit body, is already available in the process of
releasing. No additional value is gained from adding a body message
on these commits.
PR Close#37110
Introduces infrastructure to validate configuration of the ng-dev
command at run time. Allowing for errors to be returned to the
user running the command.
PR Close#37049
As per our discussion in the dev-infra sync meeting, we don't want
to have all dependencies show up as peer dependencies. Instead, we
only want to have larger dependencies such as `typescript` or buildifier
as peer dependencies. Tslib is also included for the sake of it being
generally a peer dependency of all Angular framework packages.
The rationale is that Yarn is smart enough to collapse packages
if all satisfy a given range. This means that we don't necessarily
need to have all dependencies as peer dependencies. The initial
idea was to keep all dependencies as peer dependencies so that
we have control over duplication of packages as downloading multiple
packages w/ different versions impacts local dev, CI and caches.
At the same time though, we don't want to bother with setting
up peer dependencies all the time. Not every consumer of the
shared dev-infra package would like to manually specify `yaml`
or `multimatch` etc. in the project `package.json`. Hence we
decided to go with a hybrid approach where only more impactful
dependencies are peer dependencies, and other smaller ones can
be standard depdencies that are usually collapsed by Yarn anyway.
Also this commit removes tslib from build targets that don't
rely on it.
PR Close#36980
Previously ng-dev loaded the config through a javascript file, this
change allows for the loaded file to be either javascript or
typescript. This enables configurations to be written with type
safety.
PR Close#37017
Migrating to a js file for providing a configuration allows for more
extensive configuration at run time. This allows for configs to include
logic and move beyond static values found in JSON files.
PR Close#36918
Migrates away from inline searching for files and running buildifier
directly, instead using ng-dev for formatting. Additionally, provides
a deprecation message for any usages of the previous commands.
PR Close#36842
In an effort to centralize formatting and linting enforcement into one
location, buildifier is being added as a formatter for ng-dev's format
command. Allowing for format enforcement for all .bzl, .bazel, WORKSPACE
and BUILD files.
PR Close#36842
Create a common environment stamping script in the ng-dev tooling
as to be used in common release tasks. This is the first step in
consolidating pieces of the release scripting process into a single
location to develop a release tool.
PR Close#36844
Previously we used gulp to run our formatter, currently clang-format,
across our repository. This new tool within ng-dev allows us to
migrate away from our gulp based solution as our gulp solution had
issue with memory pressure and would cause OOM errors with too large
of change sets.
PR Close#36726
* Set up dev-infra's :npm_package to also contain benchmarking suite
* Add benchmarking deps to dev-infra's package.json
* Add a bazel workspace to dev-infra's package.json. This is so that when a
project wants to use dev-infra's code and macros, they can just import the
macros from their node_modules instead of loading it separately
PR Close#36434
* Move tools/brotli-cli, tools/browsers, tools/components,
tools/ng_rollup_bundle, and modules/e2e_util to dev-infra/benchmarking
* Fix imports and references to moved folders and files
* Set up BUILD.bazel files for moved folders so they can be packaged with
dev-infra's :npm_package
PR Close#36434
Enforces a requirement that all PR commit messages contain a body
of at least 100 characters. This is meant to encourage commits
within the repo to be more descriptive of each change.
PR Close#36632
Previously, the commit message body regex only matched the first line
of the body. This change corrects the regex to match the entire line.
PR Close#36632
Currently, when verifying our pullapprove configuration, we don't
respect modifications to the set of files in a condition.
e.g. It's not possible to do the following:
```
contains_any_globs(files.exclude(...), [
```
This prevents us from having codeowner groups which match a directory,
but want to filter out specific sub directories. For example, `fw-core`
matches all files in the core package. We want to exclude the schematics
from that glob. Usually we do this by another exclude condition.
This has a *significant* downside though. It means that fw-core will not
be requested if a PR changes schematic code, _and_ actual fw-core code.
To support these conditions, the pullapprove verification tool is
refactored, so that it no longer uses Regular expressions for parsing,
but rather evaluates the code through a dynamic function. This is
possible since the conditions are written in simple Python that can
be run in NodeJS too (with small modifications/transformations).
PR Close#36661
This commit fixes an issue where adding `fixup` commits was triggering a lint error. The problem was caused by the fact that we used the entire message body while checking whether `fixup` commit has a corresponding "parent" commit in a range. This issue was found after enforcing a check that exits the process if there is an invalid commit message found (4341743b4a).
PR Close#36733
Currently the `commit-message` validation script does not exit
with a non-zero exit code if the commit message validation failed.
This means that invalid commit messages are currently not
causing CI to be red. See: https://circleci.com/gh/angular/angular/686008
PR Close#36723
Previously, the `pre-commit-validate` command (used in the `commit-msg`
git hook) assumed that the commit message was stored in
`.git/COMMIT_EDITMSG` file. This is usually true, but not when using
[git worktrees](https://git-scm.com/docs/git-worktree), where `.git` is
a file containing the path to the actual git directory.
This commit fixes it by taking advantage of the fact that git passes the
actual path of the file holding the commit message to the `commit-msg`
hook and husky exposes the arguments passed by git as
`$HUSKY_GIT_PARAMS`.
NOTE:
We cannot use the environment variable directly in the `commit-msg` hook
command, because environment variables need to be referenced differently
on Windows (`%VAR_NAME%`) vs macOS/Linux (`$VAR_NAME`). Instead, we pass
the name of the environment variable and the validation script reads the
variable's value off of `process.env`.
PR Close#36507
Currently the golden output of the circular-deps tool is purely
based on the order of source files passed to the tool, and on the
amount of imports inside source files.
This is actually resulting in deterministic output as running
the tool multiple times without any changes to source files,
results in the same output.
Though it seems like the tool is too strict and we can avoid
unnecessary golden changes if:
1. A source file that is part of a cycle is imported earlier (in terms
of how the analyzer visits them). This could result in the cycle path
starting with a different source file.
2. Source files which are not part of a cycle are imported earlier
(in terms of how the analyzer visits them). This could result in moved
items in the golden if re-approved (even though the cycles remain the same)
To fix this, we normalize the cycle path array that serves as
serializable data structure for the text-based goldens. Since
the paths represents a cycle, the path can be shifted in a
deterministic way so that cycles don't change unnecessarily
in the golden, and to simplify comparison of cycles.
Additionally, we sort the cycles in a deterministic way so
that the golden doesn't change unnecessarily (as explained above).
PR Close#36505
Prior to this change we manage a local version of commit message validation
in addition to the commit message validation tool contained in the ng-dev
tooling. By adding the ability to validate a range of commit messages
together, the remaining piece of commit message validation that is in the
local version is replicated.
We use both commands provided by the `ng-dev commit-message` tooling:
- pre-commit-validate: Set to automatically run on an git hook to validate
commits as they are created locally.
- validate-range: Run by CI for every PR, testing that all of the commits
added by the PR are valid when considered together. Ensuring that all
fixups are matched to another commit in the change.
PR Close#36172
George Kalpakas
Andrew Scott
Bjarki
Greg Magolan
Joey Perrott
Andrew Kushnir
Paul Gschwendtner
Wagner Maciel
Ikko Ashimine
Andrew Scott
Igor Minar
JiaLiPassion