Files
angular__angular/CODE_REVIEWS.md
T
Paul Gschwendtner 6d7f417392 build: restructure demo-app in favor of bazel (#13965)
Restructures the demo-app by no longer mixing up the `demo-app` name.

Currently the `demo-app` consists of two parts:

  * The demo-app
  * And the accessibility demos

As you can tell above, this is pretty much confusing because it's
technically just consisting of:

  * Accessibility demos
  * Library demos

This change cleans up the naming and makes sure that the modules are
structured in a best-practice way.

It's odd to have the `DemoAppModule` (*not the root module*) that
includes all library demos as a folder next to the actual demo
components.

This makes it extremely hard to convert the demo-app to Bazel because
we cannot target all `.ts` files through `glob` because it only matches
files inside of the same folder (or Bazel package).

In order to make it easy to run the demo-app with Bazel, and to just
clean it up, the folder structure should be as followed:

```
src/a11y-demo/
  | ...

src/dev-app/
  | ${DEMO_COMPONENT}
     | ...
     | BUILD.bazel
  | index.html
  | BUILD.bazel
```
2018-11-15 07:50:46 -08:00

1.7 KiB

Code reviews for Angular Material

  • Before any coding begins on new, large, or breaking work, a design discussion should take place.
  • All code changes require a review and approval.
  • All behaviors should be covered by unit tests in the same PR.
  • Large changes should be accompanied by corresponding e2e tests in the same PR.
  • Authors should attempt to keep PRs to 200 - 300 line changes.

Workflow

  1. The code author sends a PR for review. This request should include:
  • A high-level description of the change being made.
  • Links to any relevant issues.
  • Screenshots (for visual changes or new additions)
  1. Reviews provide comments and the author responds / makes changes. Repeat until LGTM.
  2. One or more of the reviewers applies the "LGTM" label.
  3. Once the LGTM label is applied, either the author or the reviewer can add the "merge-ready" label to indicate that the PR is ready to be merged.
  4. The party responsible for merging PRs will do so.

How PRs are merged

The team has a weekly rotation for the "caretaker" who is responsible for merging PRs. Before being merged, the caretaker runs PRs through Google's internal presubmit system. This process helps greatly in keeping the library stable by running against the tests of many applications inside of Google. Due to the volume of tests involved, this process means that there can be some delay between a PR being approved and it being merged.

The "merge safe" label means that the change doesn't affect the library itself (or the dev-app), and thus can be merged without this extra presubmit.

The "presubmit failure" label means that the PR has encountered some failure during presubmit and needs further investigation by the team.