-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create a PR template to help improve ease of reviewing #4002
Comments
Proposed template ## Why this change ?
<!---
Explain here why you want to add this change to Helix.
This can be as simple as "Fixes #issue" or as complex as you need it to be, for refactoring or deeper issues with design tradeoffs.
For theme and UI changes, post screenshots of the new look (and the old one if relevant).
If your change does not match one of the predefined categories or need a longer explanation, remove the list and write your description.
--->
<!--- Add issue number in the format "Closes #issue" to make GitHub automatically close it --->
- [ ] Bug fix
- [ ] Theme changed/added
- [ ] Language changed/added (highlights, queries, injections, config in `languages.toml` ...)
- [ ] UI change
- [ ] Feature <!--- Will very often need a longer description and a doc update --->
- [ ] Refactor
-----
## Tasks (not applicable to all PRs)
If something is unneeded, just don't check it, if it was needed, either the CI or a reviewer will tell you 😄.
<!--- Especially needed if you introduce a new feature or language --->
- [ ] *(Doc)* I ran `cargo xtask docgen` to ensure the generated documentation is up-to-date
- [ ] *(Doc)* I updated the user-facing documentation under `book/`
- [ ] *(Themes)* I ran `cargo xtask themelint` <!--- For when you add/modify a new theme --->
- [ ] *(Languages)* I ran `cargo xtask query-check` <!--- For when you add/modify a new language --->
## Questions
<!--- Do you have points you're not sure about ? Add them here ---> RenderedWhy this change ?
Tasks (not applicable to all PRs)If something is unneeded, just don't check it, if it was needed, either the CI or a reviewer will tell you 😄.
Questions |
Should this perhaps get a bump? Looking through PRs (eg. when reading release logs) is quite hilariously unsatisfactory. Like it's not just low effort, it's literally zero effort in a lot of them (description is blank)... (: One straight forward template I added for my team is: # TLDR
Updated theme `dark_plus` with
correct highlight color.
## Problem
The color for highlights did
not match the original theme.
**<image with old color>**
## Solution
Updated highlight color from
**<old color>** to **<new color>**.
**<image with new color>** I intentionally chose a quite trivial example, which at the same time would bring huge improvements to the (in this case) totally blank PRs with similar changes. Usually it takes about 20-30 sec to fill it in, a couple of minutes for more complex changes. Point being: The "extra time/to much effort" argument does not apply (I'm confident in saying that since we've pushed through hundreds of PRs using this template). Now the main benefit: Anyways, just a suggestion to get the ball rolling. |
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
As @kirawi described in this comment, making a PR template to have contributors explain the functionality and motivation of their contribution could help make reviewing PRs easier/faster, which is something that seems to be becoming increasingly necessary with the rate at which PRs being opened.
The text was updated successfully, but these errors were encountered: