-
-
Notifications
You must be signed in to change notification settings - Fork 31
Dev: Pull Requests
Pablo Mayrgundter edited this page Feb 8, 2024
·
8 revisions
A good pull request will be:
- Focused: one logical/holistic change. Less is more.
- Clear: Well-factored, commented, no debugging, etc.
- Good commit messages. "a commit message shows whether a developer is a good collaborator"
- Tested: focused and clear unit tests that exercise the key changes in your PR.
- Demonstrated: a link to a live demo of your PR.
- PR Creator is responsible for status throughout the PR lifecycle
- PR Assignee is who is being asked to do the review. Other reviewers may be added as cc's.
- Always set labels and Project (Project should be bldrs.ai - March 2023).
- Request a review from one or two of the editors of the files you're editing. Prefer recent editors. Include at least one Project Lead or Admin, see Teams.
- The Reviewer should quickly evaluate the overall status and functionality of the PR and request any major changes.
- If the PR isn't ready yet (it doesn't meet the criteria above), please ask the Creator to switch it to a Draft PR
- Draft PRs can stay in the review queue for up to 3 weeks, then we'll ask the Creator to close it and come back with a new approach later.
- Please use review comments to track status of changes
- Say "PTAL" (please take another look) to signal new changes are ready for review
- Check that all prior comments are addressed
- Comment creator closes the comment when it's addressed
- Please check minimize PR discussion on Discord, preferring to have all status in the PR comments.
- After Review Approval, Merges will happen by the PR creator if they're an Admin, they should merge or close the PR
- Outside collaborators should feel free to submit or close their own PR, and Admins will merge.
If the PR is a work-in-progress, open it as a Draft. Drafts may have debugging code enabled, tests disabled, etc..
Please keep PRs in draft until code is ready to be committed. This means debugging code removed, tests re-enabled, etc..