-
Notifications
You must be signed in to change notification settings - Fork 0
Guidelines to write a good pull request description
Code review is important. The developer is often so immersed in the change that it is easy to miss some important detail or some relevant code path, and a good code review can help you with that. A reviewer can also bring in a different perspective on a code change that can help to improve code quality or performance. Uniformity across the code base is another important consequence of code review.
It is in your best interest to do a good job describing your change in English and guide the code reviewer through your changes, especially if it encompasses a large number of line changes. It also helps the developer reason about the changes and explain in words what it is that the change is trying to achieve.
There isn't a single way to describe a pull request well, but here are some common points that I have observed over time:
- In What the code does, make sure to highlight the class or classes that the reviewer should start with and focus on. It is often the case that there is a handful of classes that the reviewer should focus on, and the other changes are simply reflecting changes from those classes. List those core classes and say explicitly what they do. This is much better than having the reviewer linearly scan 40 files because the reviewer can move directly to what matters.
- Break the description into paragraphs. They should be organized in a way that they introduce some logical pauses to the flow. There is no right or wrong, but typically, neither very long nor very short paragraphs are good ones.
- Summarize the changes in 2-3 bullets for the change log in the beginning. Those bullet points also give the reviewer a bit more context beyond the title.
- Use formatting rules, like code blocks, italics, bold, etc. It helps the reviewer to parse the information and get the right message. I often find myself having to read a description x > 2 times to understand what it is that it is trying to say because the text is flat and there is no clear structure or formatting. Don't underestimate the effect of properly formatting your text.
Consider adding some detail on how to validate the change. For example, for a bug fix, we might say how to reproduce the problem to validate the change.
Pravega - Streaming as a new software defined storage primitive
- Contributing
- Guidelines for committers
- Testing
-
Pravega Design Documents (PDPs)
- PDP-19: Retention
- PDP-20: Txn timeouts
- PDP-21: Protocol revisioning
- PDP-22: Bookkeeper based Tier-2
- PDP-23: Pravega Security
- PDP-24: Rolling transactions
- PDP-25: Read-Only Segment Store
- PDP-26: Ingestion Watermarks
- PDP-27: Admin Tools
- PDP-28: Cross routing key ordering
- PDP-29: Tables
- PDP-30: Byte Stream API
- PDP-31: End-to-end Request Tags
- PDP-32: Controller Metadata Scalability
- PDP-33: Watermarking
- PDP-34: Simplified-Tier-2
- PDP-35: Move controller metadata to KVS
- PDP-36: Connection pooling
- PDP-37: Server-side compression
- PDP-38: Schema Registry
- PDP-39: Key-Value Tables
- PDP-40: Consistent order guarantees for storage flushes
- PDP-41: Enabling Transport Layer Security (TLS) for External Clients
- PDP-42: New Resource String Format for Authorization
- PDP-43: Large Events
- PDP-44: Lightweight Transactions
- PDP-45: Healthcheck
- PDP-46: Read Only Permissions For Reading Data
- PDP-47: Pravega Message Queues