Add pull request template checklist#196
Merged
Merged
Conversation
- Add a PR template with summary and testing checklists - Prompt contributors to confirm JS, docs, native, and benchmark validation
Contributor
📝 WalkthroughWalkthroughA new GitHub pull request template was added to standardize PR submissions with sections for summary information and testing verification checkboxes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/pull_request_template.md (1)
6-10: Consider tightening checklist wording for consistency and fewer false negatives.Two small clarity tweaks: make docs conditional (
if applicable) and optionally include canonical test commands used by this repo so contributors confirm the same execution path.Suggested wording update
## Testing - [ ] Verified no regressions and confirmed the new feature or bugfix in end-to-end JavaScript/TypeScript tests -- [ ] Updated documentation +- [ ] Updated documentation (if applicable) - [ ] Optional: Verified no regressions and confirmed the new feature or bugfix in native Pascal tests (if AST, scope, evaluator, or value types changed) - [ ] Optional: Verified no benchmark regressions or confirmed benchmark coverage for the change + +<!-- Optional reference commands: +JS tests: ./build.pas testrunner && ./build/TestRunner tests +Native tests: ./build.pas clean tests && for t in build/Goccia.*.Test; do "$t"; done +-->Based on learnings: Always verify JavaScript tests by running
./build.pas testrunner && ./build/TestRunner tests, and run native Pascal tests with./build.pas clean tests && for t in build/Goccia.*.Test; do "$t"; donewhen AST/scope/evaluator/value types change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/pull_request_template.md around lines 6 - 10, Update the "Testing" checklist to reduce false negatives by making documentation and native-Pascal test items conditional and by adding the canonical test commands for JavaScript/TypeScript and native Pascal to ensure contributors run the same verification; specifically, change "Updated documentation" to "Updated documentation (if applicable)", change the native test line to "Optional: Verified no regressions and confirmed the new feature or bugfix in native Pascal tests (if applicable, e.g., AST/scope/evaluator/value type changes)", and append the recommended commands for JS/TS tests ("./build.pas testrunner && ./build/TestRunner tests") and for native Pascal tests ("./build.pas clean tests && for t in build/Goccia.*.Test; do \"$t\"; done") to the respective checklist items so contributors know the exact commands to run.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/pull_request_template.md:
- Around line 6-10: Update the "Testing" checklist to reduce false negatives by
making documentation and native-Pascal test items conditional and by adding the
canonical test commands for JavaScript/TypeScript and native Pascal to ensure
contributors run the same verification; specifically, change "Updated
documentation" to "Updated documentation (if applicable)", change the native
test line to "Optional: Verified no regressions and confirmed the new feature or
bugfix in native Pascal tests (if applicable, e.g., AST/scope/evaluator/value
type changes)", and append the recommended commands for JS/TS tests
("./build.pas testrunner && ./build/TestRunner tests") and for native Pascal
tests ("./build.pas clean tests && for t in build/Goccia.*.Test; do \"$t\";
done") to the respective checklist items so contributors know the exact commands
to run.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e7c68872-f6cf-46bb-8185-fbc8bbe88f51
📒 Files selected for processing (1)
.github/pull_request_template.md
Contributor
Benchmark Results274 benchmarks Interpreted: 🟢 28 improved · 🔴 151 regressed · 95 unchanged · avg -2.1% arraybuffer.js — Interp: 🟢 7, 7 unch. · avg +1.0% · Bytecode: 🟢 3, 🔴 4, 7 unch. · avg -0.7%
arrays.js — Interp: 🔴 13, 6 unch. · avg -1.0% · Bytecode: 🟢 8, 🔴 3, 8 unch. · avg +1.8%
async-await.js — Interp: 🔴 3, 3 unch. · avg -0.6% · Bytecode: 🟢 2, 4 unch. · avg +0.6%
classes.js — Interp: 🟢 2, 🔴 12, 17 unch. · avg -0.6% · Bytecode: 🟢 12, 19 unch. · avg +3.6%
closures.js — Interp: 🔴 4, 7 unch. · avg -0.5% · Bytecode: 🟢 4, 🔴 1, 6 unch. · avg +0.6%
collections.js — Interp: 🟢 1, 🔴 1, 10 unch. · avg -0.2% · Bytecode: 🟢 11, 🔴 1 · avg +2.5%
destructuring.js — Interp: 🟢 5, 🔴 2, 15 unch. · avg +0.3% · Bytecode: 🟢 20, 2 unch. · avg +5.8%
fibonacci.js — Interp: 🟢 3, 5 unch. · avg +1.2% · Bytecode: 🟢 5, 3 unch. · avg +2.8%
for-of.js — Interp: 🟢 4, 3 unch. · avg +1.6% · Bytecode: 🟢 6, 1 unch. · avg +4.2%
helpers/bench-module.js — Interp: 0 · Bytecode: 0
iterators.js — Interp: 🔴 14, 6 unch. · avg -2.4% · Bytecode: 🟢 18, 🔴 1, 1 unch. · avg +5.3%
json.js — Interp: 🟢 3, 🔴 11, 6 unch. · avg -1.8% · Bytecode: 🟢 19, 1 unch. · avg +5.4%
jsx.jsx — Interp: 🔴 21 · avg -5.0% · Bytecode: 🟢 19, 2 unch. · avg +3.8%
modules.js — Interp: 🔴 7, 2 unch. · avg -2.6% · Bytecode: 🟢 7, 🔴 2 · avg +0.8%
numbers.js — Interp: 🟢 1, 🔴 6, 4 unch. · avg -1.6% · Bytecode: 🟢 4, 🔴 1, 6 unch. · avg +0.4%
objects.js — Interp: 🔴 7 · avg -6.8% · Bytecode: 🟢 4, 3 unch. · avg +3.1%
promises.js — Interp: 🔴 12 · avg -8.2% · Bytecode: 🟢 4, 8 unch. · avg +1.0%
regexp.js — Interp: 🟢 1, 🔴 7, 3 unch. · avg -2.2% · Bytecode: 🟢 6, 🔴 1, 4 unch. · avg +2.7%
strings.js — Interp: 🟢 1, 🔴 9, 1 unch. · avg -4.4% · Bytecode: 🟢 8, 3 unch. · avg +3.0%
typed-arrays.js — Interp: 🔴 22 · avg -5.1% · Bytecode: 🟢 9, 🔴 1, 12 unch. · avg +2.7%
Measured on ubuntu-latest x64. Benchmark ranges compare cached main-branch min/max ops/sec with the PR run; overlapping ranges are treated as unchanged noise. Percentage deltas are secondary context. |
Contributor
Suite Timing
Measured on ubuntu-latest x64. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
.github/pull_request_template.md.Testing
Summary by CodeRabbit