T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Update PR template and dev process docs (#6158)

Browse Source 
Overall, I'm just trying to make these bits of documentation reflect our
process as it stands today. There are some specific changes I want to
draw attention to though.

Asking new contributors to pick a reviewer is a waste of time for two
reasons: Only people with write access to the repository are allowed to
pick reviewers, and new contributors have no idea who would be a good
reviewer for their PR anyway. So I'm deleting all mention of that. We
now auto-assign reviewers instead.

By the time someone is opening a PR, asking them to open an issue just
makes extra work for everyone. They've already picked an approach
without discussing it; we might as well look at what they did. We may
then have to ask them to take a different approach, but at that point,
asking them to open an issue won't save them any effort.

I removed mention of tests from the pull request template. There are
many things we'd like to see in a PR, and we may have to ask for them
during review if the contributor doesn't follow our development process
documentation. But I think the only crucial information for starting a
review is the two questions I'm leaving in the template: why do you want
this, and where can I find more context?

The code of conduct link still had the branch name as `master`, which is
a hint at how long it's been since anyone reviewed it.
This commit is contained in:
Jamey Sharp
2023-04-05 13:47:02 -07:00
committed by GitHub
parent 2d34dbef4b
commit 4d5eaea6dc
2 changed files with 35 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
.github/pull_request_template.md vendored
View File

@@ -1,18 +1,15 @@
<!-- <!--
Please make sure you include the following information:
Please ensure that the following steps are all taken care of before submitting - If this work has been discussed elsewhere, please include a link to that
the PR. conversation. If it was discussed in an issue, just mention "issue #...".
- [ ] This has been discussed in issue #..., or if not, please tell us why - Explain why this change is needed. If the details are in an issue already,
here. this can be brief.
- [ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.
- [ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.
Please ensure all communication adheres to the [code of Our development process is documented in the Wasmtime book:
conduct](https://github.com/bytecodealliance/wasmtime/blob/master/CODE_OF_CONDUCT.md). https://docs.wasmtime.dev/contributing-development-process.html
Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
--> -->

44
docs/contributing-development-process.md
View File

@@ -15,9 +15,8 @@ Our issue templates might help you through the process.
### When submitting PRs ### When submitting PRs
- Please fill in the pull request template as appropriate. It is usually - Please answer the questions in the pull request template. They are the
helpful, it speeds up the review process and helps understanding the changes minimum information we need to know in order to understand your changes.
brought by the PR.
- Write clear commit messages that start with a one-line summary of the - Write clear commit messages that start with a one-line summary of the
change (and if it's difficult to summarize in one line, consider change (and if it's difficult to summarize in one line, consider
@@ -26,27 +25,35 @@ Our issue templates might help you through the process.
code are affected, which features are affected, and anything that code are affected, which features are affected, and anything that
reviewers might want to pay special attention to. reviewers might want to pay special attention to.
- If there is code which needs explanation, prefer to put the explanation in - If there is code which needs explanation, prefer to put the explanation in a
a comment in the code, or in documentation, rather than in the commit comment in the code, or in documentation, rather than in the commit message.
message. Commit messages should explain why the new version is better than the old.
- Please include new test cases that cover your changes, if you can. If you're
not sure how to do that, we'll help you during our review process.
- For pull requests that fix existing issues, use [issue keywords]. Note that - For pull requests that fix existing issues, use [issue keywords]. Note that
not all pull requests need to have accompanying issues. not all pull requests need to have accompanying issues.
- Assign the review to somebody from the [Core Team], either using suggestions
in the list proposed by Github, or somebody else if you have a specific
person in mind.
- When updating your pull request, please make sure to re-request review if - When updating your pull request, please make sure to re-request review if
the request has been cancelled. the request has been cancelled.
### Focused commits or squashing ### Focused commits or squashing
We generally squash sequences of incremental-development commits together into We are not picky about how your git commits are structured. When we merge your
logical commits (though keeping logical commits focused). Developers may do PR, we will squash all of your commits into one, so it's okay if you add fixes
this themselves before submitting a PR or during the PR process, or Core Team in new commits.
members may do it when merging a PR. Ideally, the continuous-integration tests
should pass at each logical commit. We appreciate it if you can organize your work into separate commits which each
make one focused change, because then we can more easily understand your
changes during review. But we don't require this.
Once someone has reviewed your PR, it's easier for us if you _don't_ rebase it
when making further changes. Instead, at that point we prefer that you make new
commits on top of the already-reviewed work.
That said, sometimes we may need to ask you to rebase for various technical
reasons. If you need help doing that, please ask!
### Review and merge ### Review and merge
@@ -54,10 +61,9 @@ Anyone may submit a pull request, and anyone may comment on or review others'
pull requests. However, one review from somebody in the [Core Team] is required pull requests. However, one review from somebody in the [Core Team] is required
before the Core Team merges it. before the Core Team merges it.
Even Core Team members should create PRs for every change, including minor work Even Core Team members must create PRs and get review from another Core Team
items (version bump, removing warnings, etc.): this is helpful to keep track of member for every change, including minor work items such as version bumps,
what has happened on the repository. Very minor changes may be merged without a removing warnings, etc.
review, although it is always preferred to have one.
[issues]: https://guides.github.com/features/issues/ [issues]: https://guides.github.com/features/issues/
[pull requests]: https://help.github.com/articles/about-pull-requests/ [pull requests]: https://help.github.com/articles/about-pull-requests/