mirror of
https://github.com/qTox/qTox.git
synced 2024-03-22 14:00:36 +08:00
docs: move Reviews
section and add Testing PRs
section
Since anyone, not just maintainers can review PRs.
This commit is contained in:
parent
1c2b271716
commit
ce9406c959
|
@ -271,6 +271,55 @@ Include every section of the body that is relevant for your commit.
|
|||
space or two newlines. The rest of the commit message is then used for this.
|
||||
|
||||
|
||||
## Reviews
|
||||
|
||||
Currently `reviewable.io` is being used to review changes that land in qTox.
|
||||
|
||||
How to review:
|
||||
|
||||
1. Click on the `Reviewable` button in [pull request].
|
||||
2. Once Reviewable opens, comment on the lines that need changes.
|
||||
3. Mark as reviewed only those files that don't require any changes – this
|
||||
makes it easier to see which files need to be changed & reviewed again once
|
||||
change is made.
|
||||
4. If pull request is good to be merged, press `LGTM` button in Reviewable.
|
||||
5. Once you're done with evaluating PR, press `Publish` to make comments
|
||||
visible on GitHub.
|
||||
|
||||
When responding to review:
|
||||
|
||||
1. Click on the `Reviewable` button in [pull request].
|
||||
2. Once you push changes to the pull request, make drafts of responses to the
|
||||
change requests.
|
||||
- if you're just informing that you've made a requested change, use
|
||||
`Reviewable`'s provided `Done` button.
|
||||
- if you want discuss the change, write a response draft.
|
||||
3. When discussion points are addressed, press `Publish` button to make
|
||||
response visible on GitHub.
|
||||
|
||||
Note:
|
||||
|
||||
* when no one is assigned to the PR, *anyone* can review it
|
||||
* when there are assigned people, only they can mark review as passed
|
||||
|
||||
### Testing PRs
|
||||
|
||||
The easiest way is to use [`test-pr.sh`] script to get PR merged on top of
|
||||
current `master`. E.g. to get pull request `#1234`:
|
||||
|
||||
```bash
|
||||
./test-pr.sh 1234
|
||||
```
|
||||
|
||||
That should create branches named `1234` and `test1234`. `test1234` is what you
|
||||
would want to test. If script fails to merge branch because of conflicts, fret
|
||||
not, it doesn't need testing until PR author fixes merge conflicts. You might
|
||||
want to leave a comment on the PR saying that it needs a rebase :smile:
|
||||
|
||||
As for testing itself, there's a nice entry on the wiki:
|
||||
https://github.com/qTox/qTox/wiki/Testing
|
||||
|
||||
|
||||
## Git config
|
||||
|
||||
*Not a requirement, just a friendly tip. :wink:*
|
||||
|
@ -440,3 +489,7 @@ https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.a
|
|||
Symbols that should be forbidden for filenames under Windows:
|
||||
|
||||
`<` `>` `:` `"` `/` `\` `|` `?` `*`
|
||||
|
||||
|
||||
[pull request]: https://github.com/qTox/qTox/pulls
|
||||
[`test-pr.sh`]: /test-pr.sh
|
||||
|
|
|
@ -53,6 +53,11 @@ git config --global alias.logs 'log --show-signature'
|
|||
be signed, and websites can fairly well mess things up.
|
||||
- **always** test PR that is being merged.
|
||||
- **always** GPG-sign PR that you're merging.
|
||||
|
||||
Commits that are about to be merged don't have to be signed, but the
|
||||
merge-commit **must** be signed. To simplify the process, and ensure that
|
||||
things are done "right", it's preferable to use the [`merge-pr.sh`] script,
|
||||
which does that for you automatically.
|
||||
- **use** [`merge-pr.sh`] script to merge PRs, e.g. `./merge-pr.sh 1234`.
|
||||
|
||||
You don't have to use it, but then you're running into risk of breaking
|
||||
|
@ -61,11 +66,6 @@ git config --global alias.logs 'log --show-signature'
|
|||
|
||||
Risk, that can be avoided when one doesn't type manually merge message :wink:
|
||||
- **might want** to use [`test-pr.sh`].
|
||||
|
||||
Commits that are about to be merged don't have to be signed, but merge-commit
|
||||
**must** be signed. To simplify process, and ensure that things are done
|
||||
"right", it's preferable to use [`merge-pr.sh`] script, which does that for
|
||||
you automatically.
|
||||
- give a PR some "breathing space" right after it's created – i.e. merging
|
||||
something right away can lead to bugs & regressions suddenly popping up, thus
|
||||
it's preferable to wait at least a day or so, to let people test & comment on
|
||||
|
@ -80,26 +80,6 @@ git config --global alias.logs 'log --show-signature'
|
|||
- if PR doesn't apply properly on top of current master (when using
|
||||
[`merge-pr.sh`] script), request a rebase and tag PR with `PR-needs-rebase`.
|
||||
|
||||
## Reviews
|
||||
|
||||
Currently `reviewable.io` is being used to review changes that land in qTox.
|
||||
|
||||
How to review:
|
||||
|
||||
1. Click on the `Reviewable` button in pull request.
|
||||
2. Once Reviewable opens, comment on the lines that need changes.
|
||||
3. Mark as reviewed only those files that don't require any changes – this
|
||||
makes it easier to see which files need to be changed & reviewed again once
|
||||
change is made.
|
||||
4. If pull request is good to be merged, press `LGTM` button in Reviewable.
|
||||
5. Once you're done with evaluating PR, press `Publish` to make comments
|
||||
visible on GitHub.
|
||||
|
||||
Note:
|
||||
|
||||
* when no one is assigned to the PR, *anyone* can review it
|
||||
* when there are assigned people, only they can mark review as passed
|
||||
|
||||
|
||||
# Issues
|
||||
|
||||
|
|
|
@ -3,11 +3,10 @@ qTox
|
|||
|
||||
[**User Manual**](/doc/user_manual_en.md) **|**
|
||||
[**Install**](/INSTALL.md) **|**
|
||||
[**Contribute**](https://github.com/qTox/qTox/wiki#contributing) **|**
|
||||
[**Help us**](#help-us) **|**
|
||||
[**Report bugs**](https://github.com/qTox/qTox/wiki/Writing-Useful-Bug-Reports) **|**
|
||||
[**Translate**](/translations/README.md) **|**
|
||||
[**Jenkins builds**](https://build.tox.chat/) **|**
|
||||
[**Keyboard shortcuts**](https://github.com/qTox/qTox/wiki/Keyboard-shortcuts) **|**
|
||||
[**Mailing list**](https://lists.tox.chat) **|**
|
||||
**IRC Channel:** [#qtox@freenode]
|
||||
|
||||
|
@ -106,7 +105,7 @@ There are [IRC logs] available.
|
|||
[issues that need help]: https://github.com/qTox/qTox/labels/help%20wanted
|
||||
[qTox-dev mailing list]: https://lists.tox.chat/listinfo/qtox-dev
|
||||
[reporting bugs]: https://github.com/qTox/qTox/wiki/Writing-Useful-Bug-Reports
|
||||
[Reviewing and testing pull requests]: https://github.com/qTox/qTox/pulls
|
||||
[Reviewing and testing pull requests]: /CONTRIBUTING.md#reviews
|
||||
[Roadmap]: https://github.com/qTox/qTox/milestones
|
||||
[Testing]: https://github.com/qTox/qTox/wiki/Testing
|
||||
[Translating, it's easy]: https://github.com/qTox/qTox/blob/master/translations/README.md
|
||||
|
|
Loading…
Reference in New Issue
Block a user