Document development and PR review workflows#77
Conversation
| When creating a new branch to work on changes, it is recommended to use the | ||
| format `<type>_<description>`, where <type> is `feat`, `fix`, `doc`, or a | ||
| similar descriptor of the nature of changes being contributed and <description> | ||
| is a brief description in kebab-case. For example, the branch that adds this | ||
| page to documentation is labeled `doc_developer-workflow`. | ||
| > This is similar to [Conventional Branch](https://conventionalbranch.org/), | ||
| though there are cases where `/` in a branch name can cause issues with | ||
| git ref parsing that naively assumes it can segment the complete ref on `/`. |
There was a problem hiding this comment.
I would maybe add something here about keeping branch names all lowercase, as macOS file paths are case-insensitive, so branch names that differ only in capitalization will conflict. For example, feat_this-branch will conflict locally with feat_This-Branch.
| When rebasing a branch, it is possible that merge conflicts will arise due to | ||
| changes in the base branch overlapping changes in the feature branch. When this | ||
| happens, the developer performing the rebase must make decisions about how to | ||
| resolve the conflicts. | ||
|
|
||
| Since resolving conflicts is a potentially destructive operation, it is | ||
| recommended to create a backup of the feature branch being rebased; this may | ||
| be done locally or by creating a new branch on GitHub, based on the | ||
| feature branch. With a safe backup in place, resolve the conflicts and complete | ||
| the rebase. |
There was a problem hiding this comment.
We might want to add some guidance here for how to handle merge conflicts where code written by more than one developer on a given branch has conflicts (i.e., the developer doing the rebase may have to make conflict resolution decisions about code they did not write and may not fully understand).
| ### 2. Review the Code Changes | ||
| Code changes should be reviewed for correctness, readability, consistency, and | ||
| scope. Much of the review process is subjective, so this document will only aim | ||
| to provide guidance for things to look for, rather than a rubric for evaluation. |
There was a problem hiding this comment.
Just a suggestion, but we could look at codifying this section into a standardized Agent Skill that could be added to each repo and run by the reviewer as a secondary "AI check" on the PR.
NeonCharlie-24
left a comment
NeonCharlie-24
left a comment
There was a problem hiding this comment.
This all looks good to me! Left two comments on areas I've had trouble with that may benefit from a bit more elaboration.
Unrelated to any changes that need to be made to these documents, I also left a suggestion about potentially adding a standardized Agent Skill to our PR review process.
2 participants
Description
Adds
Standard Development Workflowdocument to describe best practices around branch naming, PR management, and rebase operationsAdds
Reviewing a Pull Requestdocument to outline the PR review process and considerationsIssues
Other Notes
Open to feedback if this can be better documented or clarified