Documentation: add PR template #5851
Conversation
potatowagon
requested review from
zurfyx,
fantactuka,
acywatson,
Fetz and
ivailop7
as code owners
facebook-github-bot
added
the
CLA Signed
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
size-limit report 📦
|
|
Hi! Let's add selection of the affected modules to the PR template, so we can sort them easier. Ex: [package name] (@lexical/core). This would allow people that recently worked on the respective packages to review PRs faster. Also sorting by type would help: chore, docs, feat, bug... |
do u have an example of how this would fit into the PR template? youre welcomed to suggest a code change directly onto .github/pull_request_template.md |
.github/pull_request_template.md
Outdated
| @@ -0,0 +1,16 @@ | |||
| ## Description | |||
There was a problem hiding this comment.
| ## Description | |
| ## What type of PR is this? | |
| <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> | |
| <!-- Leave 1 option from the list: --> | |
| **Breaking change / Refactor / Feature / Bug Fix / Documentation Update / Chore** | |
| ## Description | |
| <!-- Describe the changes in this pull request --> | |
| **Current behavior:** | |
| <!-- Please describe the current behavior that you are modifying. --> | |
| **New behavior:** | |
| <!-- Please describe the behavior or changes that are being added by this PR. --> | |
| **Closes:** #<!-- issue number --> | |
| **Affected packages:** <!-- List names of the affected packages within lexical monorepo. If none affected - put "infra" here. | |
| ## Test plan | |
| ### Before | |
| *Insert relevant screenshots/recordings/automated-tests* | |
| ### After | |
| *Insert relevant screenshots/recordings/automated-tests* | |
| ## Added/updated automated tests? | |
| _We encourage you to keep the code coverage percentage at 80% and above._ | |
| - [ ] Yes | |
| - [ ] No, and this is why: _please replace this line with details on why tests | |
| have not been included_ | |
| - [ ] I need help with writing tests |
There was a problem hiding this comment.
I wonder how much this will impact velocity. I like the detailed template as a means to write down as much information as possible but a too verbose summary will impact both the writers and the readers (I often find myself re-reading old PRs to understand the context).
IMO the package name should be part of the title, the close part can be part of the description and the automated tests don't need a checkbox list.
I'd advocate for a balance between @potatowagon 's initial PR and this.
There was a problem hiding this comment.
I added a title guidance, which prompts to list PR Type and lexical packages. reason: those info would be helpful when screening through the list of pr titles of a sync diff.
For the description ive updated the guidance to include @StyleT suggestion. leaving that as guidance instead as the before/after behavior is covered in the test plan
potatowagon
force-pushed
the
pr-template
branch
from
e35c05c
to
d3acf1f
Compare
potatowagon
force-pushed
the
pr-template
branch
from
dfff928
to
24aacc1
Compare
.github/workflows/test.yml
Outdated
| @@ -129,7 +129,7 @@ jobs: | |||
| ~/Library/Caches/ms-playwright | |||
| key: ${{ runner.os }}-v${{ secrets.CACHE_VERSION }}-${{ hashFiles('package-lock.json') }} | |||
| - name: Install dependencies | |||
| if: steps.cache.outputs.cache-hit != 'true' | |||
| # if: steps.cache.outputs.cache-hit != 'true' | |||
There was a problem hiding this comment.
not planning to commit this, temp fix to workflows error
failed to load config from /Users/runner/work/lexical/lexical/packages/lexical-playground/vite.prod.config.ts
error during build:
Error:
You installed esbuild on another platform than the one you're currently using.
This won't work because esbuild is written with native code and needs to
install a platform-specific binary executable.
Specifically the "esbuild-darwin-64" package is present but this platform
needs the "esbuild-darwin-arm64" package instead. People often get into this
situation by installing esbuild on Windows or macOS and copying "node_modules"
into a Docker image that runs Linux, or by copying "node_modules" between
Windows and WSL environments.
potatowagon
force-pushed
the
pr-template
branch
from
178a369
to
8617773
Compare
https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/creating-a-pull-request-template-for-your-repository