Skip to content

Commit

Permalink
add section about copybara workflow
Browse files Browse the repository at this point in the history
  • Loading branch information
j2kun committed Mar 26, 2024
1 parent b217139 commit c14d267
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 33 deletions.
101 changes: 68 additions & 33 deletions docs/content/en/docs/contributing.md
Expand Up @@ -89,39 +89,74 @@ at the [pull request review stage](#pull-request-review-flow).
### Pull request review flow

1. **New PR**:

- When a new PR is submitted, it is inspected for quality requirements, such as
the CLA requirement, and a sufficient PR description.
- If the PR passes checks, we assign a reviewer. If not, we request additional
changes to ensure the PR passes CI checks.

- When a new PR is submitted, it is inspected for quality requirements, such as
the CLA requirement, and a sufficient PR description.
- If the PR passes checks, we assign a reviewer. If not, we request additional
changes to ensure the PR passes CI checks.
2. **Review**

- A reviewer will check the PR and potentially request additional changes.
- If a change is needed, the contributor is requested to make a suggested
change. Please make changes with additional commits to your PR, to ensure that
the reviewer can easily see the diff.
- If all looks good, the reviewer will approve the PR.
- This cycle repeats itself until the PR is approved.

- A reviewer will check the PR and potentially request additional changes.
- If a change is needed, the contributor is requested to make a suggested
change. Please make changes with additional commits to your PR, to ensure that
the reviewer can easily see the diff.
- If all looks good, the reviewer will approve the PR.
- This cycle repeats itself until the PR is approved.
3. **Approved**

- **At this stage, you must squash your commits into a single commit.**
- Once the PR is approved, a GitHub workflow will
[check](https://github.com/google/heir/blob/main/.github/workflows/pr_review.yml)
your PR for multiple commits. You may use the `git rebase -i` to squash the
commits. Pull requests must comprise of a single git commit before merging.

- **At this stage, you must squash your commits into a single commit.**
- Once the PR is approved, a GitHub workflow will
[check](https://github.com/google/heir/blob/main/.github/workflows/pr_review.yml)
your PR for multiple commits. You may use the `git rebase -i` to squash the
commits. Pull requests must comprise of a single git commit before merging.
4. **Pull Ready**

- Once the PR is squashed into a single git commit, a maintainer will apply the
`pull ready` label.
- This initiates the internal code migration and presubmits.
- If needed, we may come to you to make some minor changes in case tests fail at
this stage. If so, we will request changes from the GitHub UI and the PR must
be approved again.
- At times, it may not be your change, but it may be additional internal lints
and layering checks that are not integrated with `bazel` or open-source
tooling. We will go ahead and fix this.
- Once the internal tests pass, we merge the code internally and externally. You
will see `copybara-bot` close the PR and merge your commit directly into main.
- Once the PR is squashed into a single git commit, a maintainer will apply the
`pull ready` label.
- This initiates the internal code migration and presubmits.
- After the internal process is finished, the commit will be added to `main`
and the PR closed as merged by that commit.

### Internal review details

This diagram summarizes the GitHub/Google code synchronization process. This
is largely automated by a Google-owned system called Copybara, the configuration
for which is Google-internal only. This system treats the Google-internal
version of HEIR as the source of truth, and applies specified transformation
rules to copy internal changes to GitHub and integrate external PRs internally.

Notable aspects:

- The final merged code may differ slightly from a PR. The changes are mainly
to support stricter internal requirements for BUILD files that we cannot
reproduce externally due to minor differences between Google's internal build
systems and bazel that we don't know how to align. Sometimes they will also
include additional code quality fixes suggested by internal static analyzers
that do not exist outside of Google.
- Due to the above, signed commits with internal modifications will not
maintain valid signatures after merging, which labels the commit with a
warning.
- You will see various actions taken on GitHub that include `copybara` in the
name, such as changes that originate from Google engineers doing various
approved migrations (e.g., migrating HEIR to support changes in MLIR or
abseil).

{{< figure title="A diagram summarizing the copybara flow for HEIR internally to Google" src="/images/hugo-copybara.png" link="/images/hugo-copybara.png" >}}

### Why bother with Copybara?

tl;dr: Automatic syncing with upstream MLIR and associated code migration.

Until HEIR has a formal governance structure in place, Google
engineers—specifically Asra Ali, Shruthi Gorantala, and Jeremy Kun—are the
codebase stewards. Because the project is young and the team is small, we want
to reduce our workload. One important aspect of that is keeping up to date
with the upstream MLIR project and incorporating bug fixes and new features
into HEIR. Google also wishes to stay up to date with MLIR and LLVM, and so
it has tooling devoted to integrating new MLIR changes into Google's monorepo
every few hours. As part of that rotation, a set of approved internal projects
that depend on MLIR (like TensorFlow) are patched to support breaking changes
in MLIR. HEIR is one of those approved projects.

As shown in the previous section, the cost of this is that no change can go
into HEIR without at least two Googlers approving it, and the project is held
to a specific set of code quality standards, namely Google's. We acknowledge
these quirks, and look forward to the day when HEIR is useful enough and
important enough that we can revisit this governance structure with the
community.
Binary file added docs/static/images/heir-copybara.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit c14d267

Please sign in to comment.