Submitting a pull request

This page offers some tips for making your pull requests easy to review. Following this advice will help the whole Zulip project move more quickly by saving maintainers time when they review your code. It will also make a big difference for getting your work integrated without delay. For a detailed overview of Zulip's PR review process, see the pull request review process guide.

Posting a pull request

Before requesting a review for your pull request, follow our guide to carefully review and test your own code. Doing so can save many review round-trips.
Make sure the pull request template is filled out correctly, and that all the relevant points on the self-review checklist (if the repository has one) have been addressed.
Be sure to explicitly call out any open questions, concerns, or decisions you are uncertain about.

Prepare your pull request

Beyond writing the code, you will need to prepare your work to make it as easy as possible for others to review. When you believe your code is ready, follow the guide on how to review code to review your own work. You can often find things you missed by taking a step back to look over your work before asking others to do so. Catching mistakes yourself will help your PRs be merged faster, and folks will appreciate the quality and professionalism of your work.

Be sure to take a careful look at your commit structure and commit messages, and do your best to follow Zulip's commit guidelines. This makes it much easier for code reviewers to spot bugs, so if your PR does not follow the guidelines, reviewers will ask you to restructure it prior to going through the code.

Submit your pull request for review

If you are new to Git, see our guide on making a pull request for detailed technical instructions on how to submit a pull request. When submitting your PR, you will need to:

Clearly describe the work you are submitting. Make sure the pull request template is filled out correctly, and that all the relevant points on the self-review checklist (if the repository has one) have been addressed. See the reviewable pull requests guide for advice.
If you have a question that you expect to be resolved during the review process, put it in a PR comment attached to a relevant part of the changes. The review process will go a lot more smoothly if points of uncertainty are explicitly laid out.
Make sure that the pull request passes all CI tests. You can sometimes request initial feedback if there are open questions that will impact how you update the tests. But in general, maintainers will wait for your PR to pass tests before reviewing your work.

If any part of your contribution is from someone else (code snippets, images, sounds, or any other copyrightable work, modified or unmodified), be sure to review the instructions on how to properly attribute the work.

If your PR was not ready for review when you first posted it (e.g., because it was failing tests, or you weren't done working through the self-review checklist), notify maintainers when you'd like them to take a look by posting a quick "Ready for review!" comment on the main GitHub thread for your PR.

Draft pull requests

If it helps your workflow, you can submit your pull request marked as a draft while you're still working on it. When ready for review:

Make sure your PR is no longer marked as a draft, and doesn't have "WIP" in the title.
Post a quick "Ready for review!" comment on the main GitHub thread for your PR.

Working on larger projects

For a larger project, aim to create a series of small (less than 100 lines of code) commits that are each safely mergeable and move you towards your goal. A mergeable commit:

Is well-tested and passes all the tests. That is, changes to tests should be in the same commit as changes to the code that they are testing.
Does not make Zulip worse. For example, it is fine to add backend capabilities without adding a frontend to access them. It's not fine to add a frontend component with no backend to make it work.

Ideally, when reviewing a branch you are working on, the maintainer 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 pull request.

Here is some advice on how to proceed:

Use git rebase -i as much as you need to shape your commit structure. See the Git guide for useful resources on mastering Git.
If you need to refactor code, add tests, rename variables, or make other changes that do not change the functionality of the product, make those changes into a series of preparatory commits that can be merged independently of building the feature itself.
To figure out what refactoring needs to happen, you might first make a hacky attempt at hooking together the feature, with reading and print statements as part of the effort, to identify any refactoring needed or tests you want to write to help make sure your changes won't break anything important as you work. Work out a fast and consistent test procedure for how to make sure the feature is working as planned.
Build a mergeable version of the feature on top of those refactorings. Whenever possible, find chunks of complexity that you can separate from the rest of the project.

See our commit discipline guide 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. Here are some additional tips and best practices that Zulip contributors and maintainers have found helpful for writing clear and thorough pull request comments:

If there has been a conversation in the Zulip development community about the changes you've made or the issue your pull request addresses, please cross-link between your pull request and those conversations. This provides helpful context for maintainers and reviewers. Specifically, it's best to link from your pull request to a specific message, as these links will still work even if the topic of the conversation is renamed, moved or resolved.

Once you've created a pull request on GitHub, you can use one of the custom linkifiers in the development community to easily link to your pull request from the relevant conversation.
For screenshots or screencasts 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 several 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 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 search for the class name(s) to confirm that you've tested all the impacted areas of the UI and/or documentation.
```
$ git grep '.example-class-name' web/templates/ templates/
templates/corporate/...
templates/zerver/...
web/templates/...
```

9.4 KiB Raw Blame History