Update Pull Request template - #59062
Conversation
MrSubidubi
left a comment
There was a problem hiding this comment.
Aside from the two comments, the only concern I somewhat have is that the template is getting rather large at this point and I'd like to at least keep in mind that this might not lead to an increase in quality of PR descriptions - if anything, I think most of our own PRs do not at all go through the list outlined here, and I doubt this will change with more steps inbetween.
Still do think this is an overall improvement, curious though whether we should add something amongst the lines of "Thanks for taking the time to fill the template out, it helps a lot and we appreciate it" - this technicaly goes against what I mentioned above with this being quite long, but feels better to me than just be outright demanding. Curious about others thoughts.
| - If this PR includes a visual change, consider adding a screenshot, GIF, or video | ||
| - A before/after comparison is very useful for changes to existing features! | ||
|
|
||
| While a showcase should aim to be brief and digestible, you can use a toggleable section to save space on longer showcases: |
There was a problem hiding this comment.
Not sure it's worth a mention here, but I sometimes like having a markdown table with a before/after for visual changes as it's easier to have them side by side to compare frequently
In general, staff PRs aren't subject to the same requirements as community PRs at this time, and often staff PRs delete the current template entirely. While I personally believe that will have to change at some point as the company grows (and would welcome any staff who wants to partake in the template here), it is not expected that staff will engage with this template at this time in the same way the community will. Additionally, I don't think the number of steps is a dominating factor in that; the current staff culture at zed is a pairing culture and not a PR/review culture, whereas community/oss contributors by necessity have a PR/review culture.
I'm sympathetic to this but while other lines in the template need to be replaced or "answered", this would only need to be deleted in every pull request. We can just as easily thank people when engaging with them, which both feels more personal and doesn't add work to the PR template that doesn't serve the goal. |
# Objective Give a bit more structure to contributors so that each (potentially new) contributor doesn't have to come up with their own structure on-the-fly. ## Solution Update the PR template to include sections for describing - what the PR is trying to achieve - how it decided to achieve the goal - how reviewers can test the work - demos of the new functionality ## Testing I used the PR template for this PR, so reviewers can evaluate if they think the structure is useful. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable ## Showcase <details> <summary>Click to view showcase</summary> ### Before <img width="1082" height="572" alt="image" src="https://github.com/user-attachments/assets/bc72b8ab-f5ec-4270-b1ec-546456525e92" /> ### After <img width="1241" height="945" alt="image" src="https://github.com/user-attachments/assets/088d90e4-2cb1-42cd-9f8f-0b2783ab7fe4" /> </details> --- Release Notes: - N/A
# Objective Give a bit more structure to contributors so that each (potentially new) contributor doesn't have to come up with their own structure on-the-fly. ## Solution Update the PR template to include sections for describing - what the PR is trying to achieve - how it decided to achieve the goal - how reviewers can test the work - demos of the new functionality ## Testing I used the PR template for this PR, so reviewers can evaluate if they think the structure is useful. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable ## Showcase <details> <summary>Click to view showcase</summary> ### Before <img width="1082" height="572" alt="image" src="https://github.com/user-attachments/assets/bc72b8ab-f5ec-4270-b1ec-546456525e92" /> ### After <img width="1241" height="945" alt="image" src="https://github.com/user-attachments/assets/088d90e4-2cb1-42cd-9f8f-0b2783ab7fe4" /> </details> --- Release Notes: - N/A
Objective
Give a bit more structure to contributors so that each (potentially new) contributor doesn't have to come up with their own structure on-the-fly.
Solution
Update the PR template to include sections for describing
Testing
I used the PR template for this PR, so reviewers can evaluate if they think the structure is useful.
Self-Review Checklist:
Showcase
Click to view showcase
Before
After
Release Notes: