Update CONTRIBUTING.md with review and issue guidelines#83
Update CONTRIBUTING.md with review and issue guidelines#83Zahnentferner merged 2 commits intoAOSSIE-Org:mainfrom
Conversation
Added guidelines for reviewing PRs and issue assignment.
WalkthroughUpdates to contributor guidance: adds PR checklist and reviewer-perspective item, introduces [WIP] title guidance, new "Reviewing PRs" section to avoid duplicate work, expands code style guidance to discourage unnecessary complexity, and clarifies issue-assignment rules including a 2+ day rule for claiming issues. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CONTRIBUTING.md`:
- Around line 532-538: Fix the grammar and ambiguity in the two bullets: replace
the second bullet with a clear rule that applies only when an issue is
unassigned and has no active PRs (e.g., "If an issue is unassigned and has no
active PRs for 2 or more days, comment your intent on the issue before starting
work; after commenting you may begin if no one else does."), merge or rephrase
the "check for existing PRs" sentence to read clearly ("Check for existing PRs
before starting to avoid duplication"), and remove or convert the HTML comment
into visible guidance to avoid hidden conflicting policy; update the exact text
in the CONTRIBUTING.md section where the current bullets appear (the two lines
in the diff) accordingly.
- Around line 217-223: Replace the ambiguous sentence under "Use `[WIP]` in your
PR title for incomplete PRs." with explicit guidance for authors: state that
authors should add `[WIP]` only while actively iterating on a change, remove
`[WIP]` once the PR is feature-complete and ready for review, and include clear
exit criteria (e.g., "all tests pass, description and checklist complete, and CI
green") before removing `[WIP]`; mention that reviewers should not be blocked by
a lingering `[WIP]` and may close or request updates on stale WIP PRs.
Clarify contribution guidelines regarding issue assignment and PRs.
There was a problem hiding this comment.
♻️ Duplicate comments (4)
CONTRIBUTING.md (4)
217-217:⚠️ Potential issue | 🟡 MinorClarify
[WIP]usage and explicit exit criteria.Line 217 is still ambiguous (“Don’t use this as a way to gatekeep”). Please define when to remove
[WIP](e.g., tests pass, checklist complete, CI green).✏️ Suggested revision
-- Use `[WIP]` in your PR title for incomplete PRs. Don't use this as a way to gatekeep; focus on one change until it gets merged. +- Use `[WIP]` in your PR title only while the work is incomplete. Remove `[WIP]` once the PR is ready for review (tests pass, checklist complete, CI green). Keep scope focused to one change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CONTRIBUTING.md` at line 217, Update the “[WIP]” guidance sentence to include explicit exit criteria: replace the ambiguous phrase "Don't use this as a way to gatekeep" with a short checklist of conditions required to remove “[WIP]” (e.g., "remove [WIP] when tests pass locally and in CI, all checklist items are completed, code review approvals are received, and CI status is green"); ensure the text mentions that using “[WIP]” is fine while work is ongoing but must be removed once those criteria are met and the PR focuses on a single change.
538-538:⚠️ Potential issue | 🟡 MinorFix grammar in issue-claiming rule.
Line 538 has a grammar error: “there might PRs” → “there might be PRs.”
✏️ Suggested revision
-- Check for existing PRs before starting to avoid duplication, as there might PRs that didn't mention the related issue +- Check for existing PRs before starting to avoid duplication, as there might be PRs that didn't mention the related issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CONTRIBUTING.md` at line 538, In the issue-claiming rule sentence that currently reads "there might PRs that didn't mention the related issue", correct the grammar by inserting "be" so it reads "there might be PRs that didn't mention the related issue"; locate the sentence in the issue-claiming section (the fragment "there might PRs") and update it to "there might be PRs" to fix the grammar.
532-535:⚠️ Potential issue | 🟡 MinorAvoid hidden policy rules in HTML comments.
Lines 532-535 keep assignment rules hidden, which can create conflicting source-of-truth behavior for contributors.
✏️ Suggested revision
-<!-- -- Wait for assignment before starting work -- Issues will be reassigned if inactive for extended periods ---> +- Wait for assignment before starting work. +- Issues may be reassigned if inactive for extended periods.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CONTRIBUTING.md` around lines 532 - 535, Remove the hidden policy rules from the HTML comment block and surface them as visible contributor guidance: delete the commented lines "<!-- Wait for assignment before starting work - Issues will be reassigned if inactive for extended periods -->" and add equivalent, explicit text in the contributor-facing rules section (e.g., a new bullet under "Issue assignment" or "Contribution guidelines") so the assignment rule is not hidden; ensure the new visible copy matches the intent and formatting of surrounding guidelines and remove the HTML comment entirely to avoid conflicting sources of truth.
536-537:⚠️ Potential issue | 🟠 MajorResolve rule conflict between “2+ days” and “assigned but inactive.”
Line 536 allows starting after 2+ days without stating “unassigned,” while Line 537 forbids working on assigned issues even when inactive. These rules conflict and can block contributors.
✏️ Suggested revision
-- If there are no active PRs for an issue for 2+ days, mention your intent under the issue and begin -- Avoid working on issues which are assigned to someone, even if they are inactive +- If an issue is unassigned and has no active PRs for 2+ days, comment your intent on the issue before starting. +- Do not work on issues assigned to others unless a maintainer reassigns or explicitly approves collaboration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CONTRIBUTING.md` around lines 536 - 537, Clarify and reconcile the two contributing rules by updating the two bullet points so they explicitly state precedence and conditions: change the first rule (the "2+ days" bullet) to require that you may begin work after 2+ days only if the issue is unassigned or labeled for community help (e.g., "help wanted") and you first post your intent in the issue, and change the second rule (the "assigned but inactive" bullet) to allow starting work on an assigned issue only after attempting to contact the assignee and waiting a defined timeout (e.g., 2+ days) with no response, otherwise respect active assignments; reference the two bullets' text when making the edits so the wording clearly defines "unassigned vs assigned but inactive" and the required steps (post intent, contact assignee, wait period) to remove the conflict.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@CONTRIBUTING.md`:
- Line 217: Update the “[WIP]” guidance sentence to include explicit exit
criteria: replace the ambiguous phrase "Don't use this as a way to gatekeep"
with a short checklist of conditions required to remove “[WIP]” (e.g., "remove
[WIP] when tests pass locally and in CI, all checklist items are completed, code
review approvals are received, and CI status is green"); ensure the text
mentions that using “[WIP]” is fine while work is ongoing but must be removed
once those criteria are met and the PR focuses on a single change.
- Line 538: In the issue-claiming rule sentence that currently reads "there
might PRs that didn't mention the related issue", correct the grammar by
inserting "be" so it reads "there might be PRs that didn't mention the related
issue"; locate the sentence in the issue-claiming section (the fragment "there
might PRs") and update it to "there might be PRs" to fix the grammar.
- Around line 532-535: Remove the hidden policy rules from the HTML comment
block and surface them as visible contributor guidance: delete the commented
lines "<!-- Wait for assignment before starting work - Issues will be reassigned
if inactive for extended periods -->" and add equivalent, explicit text in the
contributor-facing rules section (e.g., a new bullet under "Issue assignment" or
"Contribution guidelines") so the assignment rule is not hidden; ensure the new
visible copy matches the intent and formatting of surrounding guidelines and
remove the HTML comment entirely to avoid conflicting sources of truth.
- Around line 536-537: Clarify and reconcile the two contributing rules by
updating the two bullet points so they explicitly state precedence and
conditions: change the first rule (the "2+ days" bullet) to require that you may
begin work after 2+ days only if the issue is unassigned or labeled for
community help (e.g., "help wanted") and you first post your intent in the
issue, and change the second rule (the "assigned but inactive" bullet) to allow
starting work on an assigned issue only after attempting to contact the assignee
and waiting a defined timeout (e.g., 2+ days) with no response, otherwise
respect active assignments; reference the two bullets' text when making the
edits so the wording clearly defines "unassigned vs assigned but inactive" and
the required steps (post intent, contact assignee, wait period) to remove the
conflict.
Summary by CodeRabbit