mirror of https://github.com/zulip/zulip.git
docs: Update documentation for tips when writing pull request comments.
Adds a 'Tips and best practices' section to the documentation on writing reviewable pull requests. Also, updates step 3 of the documentation on creating pull requests to link to the new section and to not have an out-of-date screenshot of the GitHub pull request template.
This commit is contained in:
parent
ce68911ab2
commit
a633890d87
|
@ -21,8 +21,8 @@ difference for getting your work integrated without delay.
|
|||
|
||||
## Addressing feedback
|
||||
|
||||
- When you update your PR after addressing a round of review feedback, be clear
|
||||
about which issues you've resolved (and how!).
|
||||
- When you update your pull request after addressing a round of review feedback,
|
||||
be clear about which issues you've resolved (and how!).
|
||||
|
||||
- Even more importantly, save time for your reviewers by indicating any feedback
|
||||
you _haven't_ addressed yet.
|
||||
|
@ -45,7 +45,7 @@ should be able to verify and merge the first few commits and leave
|
|||
comments on the rest. It is by far the most efficient way to do
|
||||
collaborative development, since one is constantly making progress, we
|
||||
keep branches small, and developers don't end up repeatedly reviewing
|
||||
the earlier parts of a PR.
|
||||
the earlier parts of a pull request.
|
||||
|
||||
Here is some advice on how to proceed:
|
||||
|
||||
|
@ -71,3 +71,66 @@ Here is some advice on how to proceed:
|
|||
|
||||
See our [commit discipline guide](../contributing/commit-discipline.md) for
|
||||
more details on writing reviewable commits.
|
||||
|
||||
## Tips and best practices
|
||||
|
||||
When writing comments for pull requests, it's good to be familiar with
|
||||
[GitHub's basic formatting syntax][github-syntax]. Here are some additional
|
||||
tips and best practices that Zulip contributors and maintainers have found
|
||||
helpful for writing clear and thorough pull request comments:
|
||||
|
||||
- For [screenshots or screencasts][screenshots-gifs] of changes,
|
||||
putting them in details/summary tags reduces visual clutter
|
||||
and scroll length of pull request comments. This is especially
|
||||
useful when you have various screenshots and/or screencasts to
|
||||
include in your comment as you can put each image, or group of
|
||||
images, in separate details/summary tags.
|
||||
|
||||
```
|
||||
<details>
|
||||
<summary>Descriptive summary of image</summary>
|
||||
|
||||
![uploaded-image](uploaded-file-information)
|
||||
</details>
|
||||
```
|
||||
|
||||
- For before and after images or videos of changes, using GithHub's table
|
||||
syntax renders them side-by-side for quick and clear comparison.
|
||||
While this this works well for narrow or small images, it can be hard
|
||||
to see details in large, full screen images and videos in this format.
|
||||
|
||||
Note that you can put the table syntax inside the details/summary
|
||||
tags described above as well.
|
||||
|
||||
```
|
||||
### Descriptive header for images:
|
||||
| Before | After |
|
||||
| --- | --- |
|
||||
| ![image-before](uploaded-file-information) | ![image-after](uploaded-file-information)
|
||||
```
|
||||
|
||||
- If you've updated existing documentation in your pull request,
|
||||
include a link to the current documentation above the screenshot
|
||||
of the updates. That way a reviewer can quickly access the current
|
||||
documentation while reviewing your changes.
|
||||
|
||||
```
|
||||
[Current documentation](link-to-current-documentation-page)
|
||||
![image-after](uploaded-file-information)
|
||||
```
|
||||
|
||||
- For updates or changes to CSS class rules, it's a good practice
|
||||
to include the results of a [git-grep][git-grep] search for
|
||||
the class name(s) to confirm that you've tested all the impacted
|
||||
areas of the UI and/or documentation.
|
||||
|
||||
```console
|
||||
$ git grep '.example-class-name' web/templates/ templates/
|
||||
templates/corporate/...
|
||||
templates/zerver/...
|
||||
web/templates/...
|
||||
```
|
||||
|
||||
[github-syntax]: https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax
|
||||
[git-grep]: https://git-scm.com/docs/git-grep
|
||||
[screenshots-gifs]: ../tutorials/screenshot-and-gif-software.md
|
||||
|
|
|
@ -103,34 +103,32 @@ complicated rebase.
|
|||
### Step 3: Open the pull request
|
||||
|
||||
If you've never created a pull request or need a refresher, take a look at
|
||||
GitHub's article [creating a pull request from a
|
||||
GitHub's article on [creating a pull request from a
|
||||
fork][github-help-create-pr-fork]. We'll briefly review the process here.
|
||||
|
||||
The first step in creating a pull request is to use your web browser to
|
||||
navigate to your fork of Zulip. Sign in to GitHub if you haven't already.
|
||||
First, sign in to GitHub on your web browser and navigate to your fork of Zulip.
|
||||
|
||||
Next, navigate to the branch you've been working on. Do this by clicking on the
|
||||
**Branch** button and selecting the relevant branch. Finally, click the **New
|
||||
pull request** button.
|
||||
pull request** button. Alternatively, if you've recently pushed the relevant
|
||||
branch to your fork, you will see a **Compare & pull request** button.
|
||||
|
||||
Alternatively, if you've recently pushed to your fork, you will see a green
|
||||
**Compare & pull request** button.
|
||||
A pull request template will open with some information pre-filled in.
|
||||
Provide (or update) the title for your pull request and write a first comment.
|
||||
|
||||
You'll see the _Open a pull request_ page:
|
||||
|
||||
![images-create-pr]
|
||||
|
||||
Provide a **title** and first comment for your pull request. Remember to mark
|
||||
your pull request as a [draft][github-help-draft-pr] if it is a
|
||||
work-in-progress.
|
||||
|
||||
If your pull request has an effect on the visuals of a component, you might want
|
||||
to include a screenshot of this change or a GIF of the interaction in your first
|
||||
comment. This will allow reviewers to comment on your changes without having to
|
||||
check out your branch; you can find a list of tools you can use for this over
|
||||
If your pull request has an effect on the visuals of a component, you will want
|
||||
to include a screenshot of this change or a GIF/screencast of the interaction in
|
||||
your first comment. You can find a list of tools you can use for this
|
||||
[here][screenshots-gifs].
|
||||
|
||||
When ready, click the green **Create pull request** to submit the pull request.
|
||||
See the documentation for creating [reviewable pull requests][reviewable-prs]
|
||||
for more guidance and tips when writing pull request comments. If the repository
|
||||
has a self-review checklist in the pull request template, make sure that all the
|
||||
relevant points have been addressed before submitting it.
|
||||
|
||||
When ready, click the **Create pull request** to submit the pull request.
|
||||
Remember to mark your pull request as a [draft][github-help-draft-pr] if it is a
|
||||
work-in-progress.
|
||||
|
||||
Note: **Pull request titles are different from commit messages.** Commit
|
||||
messages can be edited with `git commit --amend`, `git rebase -i`, etc., while
|
||||
|
@ -158,7 +156,7 @@ for another review.
|
|||
[github-help-about-pr]: https://help.github.com/en/articles/about-pull-requests
|
||||
[github-help-create-pr-fork]: https://help.github.com/en/articles/creating-a-pull-request-from-a-fork
|
||||
[github-help-draft-pr]: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests#draft-pull-requests
|
||||
[images-create-pr]: ../images/zulip-open-pr.png
|
||||
[keep-up-to-date]: using.md#keep-your-fork-up-to-date
|
||||
[self-push-commits]: using.md#push-your-commits-to-github
|
||||
[screenshots-gifs]: ../tutorials/screenshot-and-gif-software.md
|
||||
[reviewable-prs]: ../contributing/reviewable-prs.md
|
||||
|
|
Binary file not shown.
Before Width: | Height: | Size: 95 KiB |
Loading…
Reference in New Issue