-
Notifications
You must be signed in to change notification settings - Fork 960
Improve PR description template #7706
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
Merged
alexshtin
merged 14 commits into
temporalio:main
from
alexshtin:feature/improve-pr-template
May 6, 2025
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
786f2fa
Improve PR template
alexshtin 3947a5d
Add hotfix section
alexshtin d62aba4
Address feedback
alexshtin 3ac9a2a
GHA and addredd feedback
alexshtin 28156ef
Remove unckeded items too
alexshtin d0f2bbd
Remove CI bulletpoint
alexshtin 9aa72ee
Add placeholder check
alexshtin d29f34d
V2
alexshtin 5d66ee7
fix
alexshtin 0b68360
Fix bash
alexshtin 05805ad
Fix bash
alexshtin 4eb0358
JS is better than bash?
alexshtin 84ff611
Improve
alexshtin afc3434
Switch to placeholders
alexshtin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,15 @@ | ||
## What changed? | ||
<!-- Describe what has changed in this PR --> | ||
_Describe what has changed in this PR._ | ||
|
||
## Why? | ||
<!-- Tell your future self why have you made these changes --> | ||
_Tell your future self why have you made these changes._ | ||
|
||
## How did you test it? | ||
<!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> | ||
- [ ] built | ||
- [ ] run locally and tested manually | ||
- [ ] covered by existing tests | ||
- [ ] added new unit test(s) | ||
- [ ] added new functional test(s) | ||
|
||
## Potential risks | ||
<!-- Assuming the worst case, what can be broken when deploying this change to production? --> | ||
|
||
## Documentation | ||
<!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant | ||
new behavior is added, have you described that in `docs/`? --> | ||
|
||
## Is hotfix candidate? | ||
<!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) --> | ||
_Any change is risky. Identify all risks you are aware of. If none, remove this section._ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
name: Validate PR description for placeholder lines or empty sections | ||
|
||
on: | ||
pull_request: | ||
types: [opened, edited, synchronize, reopened] | ||
|
||
permissions: | ||
pull-requests: read | ||
|
||
jobs: | ||
validate-pr-description: | ||
runs-on: ubuntu-latest | ||
|
||
steps: | ||
- name: Validate PR description for placeholder lines or empty sections | ||
uses: actions/github-script@v7 | ||
with: | ||
script: | | ||
const pr = await github.rest.pulls.get({ | ||
owner: context.repo.owner, | ||
repo: context.repo.repo, | ||
pull_number: context.payload.pull_request.number | ||
}); | ||
|
||
const body = pr.data.body || ''; | ||
const lines = body.split(/\r?\n/); | ||
|
||
let violations = []; | ||
|
||
// Detect placeholder lines: entire line starts and ends with _ | ||
lines.forEach((line, idx) => { | ||
if (/^_.*_$/.test(line.trim())) { | ||
violations.push(`Line ${idx + 1}: Placeholder "${line.trim()}"`); | ||
} | ||
}); | ||
|
||
// Detect empty sections: look for headers like '## Why?' followed by no meaningful content | ||
const requiredSections = ['## What changed?', '## Why?', '## How did you test it?']; | ||
requiredSections.forEach((header) => { | ||
const idx = lines.findIndex(line => line.trim().toLowerCase() === header.toLowerCase()); | ||
if (idx !== -1) { | ||
let contentIdx = idx + 1; | ||
while (contentIdx < lines.length && lines[contentIdx].trim() === '') { | ||
contentIdx++; | ||
} | ||
const nextLine = lines[contentIdx]?.trim(); | ||
if (!nextLine || /^## /.test(nextLine)) { | ||
violations.push(`Section "${header}" appears to be empty.`); | ||
} | ||
} | ||
}); | ||
|
||
if (violations.length > 0) { | ||
console.log("❌ PR description issues found:"); | ||
violations.forEach(v => console.log(`- ${v}`)); | ||
core.setFailed(`PR description must not contain placeholders or empty sections.`); | ||
} else { | ||
console.log("✅ PR description passed all checks."); | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(not a serious suggestion)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"tested in production"
"my code doesn't need testing, it is flawless"
"checked by Chuck Norris"
There a bunch of good options.