forked from mirrors/jj
docs: describe how we do code reviews
This commit is contained in:
parent
de639c3ef1
commit
69f85dfd27
2 changed files with 28 additions and 1 deletions
3
.github/PULL_REQUEST_TEMPLATE.md
vendored
3
.github/PULL_REQUEST_TEMPLATE.md
vendored
|
@ -4,7 +4,8 @@ Please describe the changes in this PR in the commit message(s) instead, with
|
|||
each commit representing one logical change. Address code review comments by
|
||||
rewriting the branch rather than adding commits on top. Use force-push when
|
||||
pushing the updated branch (`jj git push` does that automatically when you
|
||||
rewrite a branch).
|
||||
rewrite a branch). Merge the PR at will once it's been approved. See
|
||||
https://github.com/martinvonz/jj/blob/main/docs/contributing.md for details.
|
||||
-->
|
||||
|
||||
# Checklist
|
||||
|
|
|
@ -25,6 +25,32 @@ use GitHub pull requests for this purpose. Consult
|
|||
[GitHub Help](https://help.github.com/articles/about-pull-requests/) for more
|
||||
information on using pull requests.
|
||||
|
||||
Unlike many GitHub projects (but like many VCS projects), we care more about the
|
||||
contents of commits than about the contents of PRs. We review each commit
|
||||
separately, and we don't squash them when the PR is ready.
|
||||
|
||||
Each commit should ideally do one thing. For example, if you need to refactor a
|
||||
function in order to add a new feature cleanly, put the refactoring in one
|
||||
commit and the new feature in a different commit. If the refactoring itself
|
||||
consists of many parts, try to separate out those into separate commits. You can
|
||||
use `jj split` to do it if you didn't realize ahead of time how it should be
|
||||
split up. Include tests and documentation in the same commit as the code the
|
||||
test and document. The commit message should describe the changes in the commit;
|
||||
the PR description can even be empty, but feel free to include a personal
|
||||
message.
|
||||
|
||||
When you address comments on a PR, don't make the changes in a commit on top (as
|
||||
is typical on GitHub). Instead, please make the changes in the appropriate
|
||||
commit. You can do that by checking out the commit (`jj checkout/new <commit>`)
|
||||
and then squash in the changes when you're done (`jj squash`). `jj git push`
|
||||
will automatically force-push the branch.
|
||||
|
||||
When your first PR has been approved, we typically give you contributor access,
|
||||
so you can address any remaining minor comments and then merge the PR yourself
|
||||
when you're ready. If you realize that some comments require non-trivial
|
||||
changes, please ask your reviewer to take another look.
|
||||
|
||||
|
||||
### Community Guidelines
|
||||
|
||||
This project follows [Google's Open Source Community
|
||||
|
|
Loading…
Reference in a new issue