|
| 1 | +# Code reviews for Angular Flex Layout |
| 2 | + |
| 3 | +* Before any coding begins on new, large, or breaking work, a design discussion should take place. |
| 4 | +* All code changes require a review and approval. |
| 5 | +* All behaviors should be covered by unit tests in the same PR. |
| 6 | +* Large changes should be accompanied by corresponding e2e tests in the same PR. |
| 7 | +* Authors should attempt to keep PRs to 200 - 300 line changes. |
| 8 | + |
| 9 | +## Workflow |
| 10 | +1. The code author sends a PR for review. This request should include: |
| 11 | + * A mention of the intended reviewer(s) (e.g., `@ThomasBurleson`) |
| 12 | + * A high-level description of the change being made. |
| 13 | + * Links to any relevant issues. |
| 14 | + * Screenshots (for visual changes or new additions) |
| 15 | +2. Reviews provide comments and the author responds / makes changes. Repeat until LGTM. |
| 16 | +3. One or more of the reviewers applies the "LGTM" label. |
| 17 | +4. Once the LGTM label is applied, either the author or the reviewer can add the "merge-ready" |
| 18 | + label to indicate that the PR is ready to be merged. |
| 19 | +5. The party responsible for merging PRs will do so. |
| 20 | + |
| 21 | +## How PRs are merged |
| 22 | +The team has a weekly rotation for the "caretaker" who is responsible for merging PRs. Before being |
| 23 | +merged, the caretaker runs PRs through Google's internal presubmit system. This process helps |
| 24 | +greatly in keeping the library stable by running against the tests of many applications inside of |
| 25 | +Google. Due to the volume of tests involved, this process means that there can be some delay |
| 26 | +between a PR being approved and it being merged. |
| 27 | + |
| 28 | +The "merge safe" label means that the change doesn't affect the library itself (or the demo-app), |
| 29 | +and thus can be merged without this extra presubmit. |
| 30 | + |
| 31 | +The "presubmit failure" label means that the PR has encountered some failure during presubmit and |
| 32 | +needs further investigation by the team. |
0 commit comments