paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-03 21:43:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

docs: Reformat review feedback section.

Browse Source 
This mostly preserves the general shape of the text, but adds a bit of
a bulleted checklist to help make it more skimmable.
This commit is contained in:
Tim Abbott
2021-03-14 19:20:04 -07:00
parent 238c2ff4fb
commit 42379a8cd1
1 changed files with 25 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
docs/contributing/code-reviewing.md
View File

@@ -23,25 +23,34 @@ Lastly, ensuring the your PR passes CI and is organized into coherent
commits would help save reviewers time, which could otherwise be used commits would help save reviewers time, which could otherwise be used
to dive right into reviewing the PR's core functionality. to dive right into reviewing the PR's core functionality.
### Guidelines for responding to a review feedback ### Responding to a review feedback
Posting a note after you updated the PR is especially beneficial for folks like Once you've received a review and resolved any feedback, it's critical
Tim, who manage their code review queue via GitHub notifications. Simply pushing to update the GitHub thread to reflect that. Best practices are to:
an update without notifying about it on GitHub indicates to the reviewer the
contributor hasn't yet resolved that feedback, and they might miss out on doing
any follow-up reviews.
The best way to address feedback posted on your PR is to reply individually to * Make sure that CI passes and the PR is rebased onto recent master.
the respective inline review comments. Adding a note in the comments on how * Post comments on each feedback thread explaining at least how you
you addressed the feedback helps the reviewers know what to look for in a resolved the feedback, as well as any other useful information
follow-up review, much more than you posting a text saying "Updated!". Also, (problems encountered, reasoning for why you picked one of several
do communicate if you notice any potential issues when addressing feedback, options, a test you added to make sure the bug won't recur, etc.).
rather than doing a suggested change blindly. * Mark any resolved threads as "resolved" in the GitHub UI, if
appropriate.
* Post a summary comment in the main feed for the PR, explaining that
this is ready for another review, and summarizing any changes from
the previous version, details on how you tested the changes, new
screenshots/etc. More detail is better than less, as long as you
take the time to write clearly.
If you think any suggestion left on the PR requires a more complex discussion, it If you resolve the feedback, but the PR has merge conflicts, CI
can be helpful to have the discussion on a topic in the Zulip development community failures, or the most recent comment is the reviewer asking you to fix
server. In case you do that, make sure to post a short comment on the GitHub PR something, it's very likely that a potential reviewer skimming your PR
linking to the conversation so they're findable from each other. will assume it isn't ready for review and move on to other work.
If you need help or think an open discussion topic requires more
feedback or a more complex discussion, move the discussion to a topic
in the Zulip development community server. Be sure to provide links
from the GitHub PR to the conversation (and vise versa) so that it's
convenient to read both conversations together.
## Principles of code review ## Principles of code review