From ce9406c9594746e855ab8d39a94be11385b3f368 Mon Sep 17 00:00:00 2001 From: Zetok Zalbavar <zetok@openmailbox.org> Date: Fri, 7 Oct 2016 14:06:22 +0100 Subject: [PATCH] docs: move `Reviews` section and add `Testing PRs` section Since anyone, not just maintainers can review PRs. --- CONTRIBUTING.md | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ MAINTAINING.md | 30 +++++----------------------- README.md | 5 ++--- 3 files changed, 60 insertions(+), 28 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 593b64bd2..e4358dbb2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -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 diff --git a/MAINTAINING.md b/MAINTAINING.md index 012e87c60..bec074394 100644 --- a/MAINTAINING.md +++ b/MAINTAINING.md @@ -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 diff --git a/README.md b/README.md index 1e52445af..667a0d8d4 100644 --- a/README.md +++ b/README.md @@ -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