-
-
Notifications
You must be signed in to change notification settings - Fork 4k
Improved Contributor Guidelines & Education (#5348, part1) #5350
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
Conversation
based on AI proposals in #5348
WalkthroughCONTRIBUTING.md is substantially reorganized: PR guidance renamed and restructured with a new Getting Started section; added a Testing Your Changes checklist; expanded PR description, pre-submission checks, and update practices (ongoing commits allowed; squashing not required); added During Review, Responding to Reviews, Dealing with Merge Conflicts (GUI/CLI and cherry-pick examples), Additional Resources, After Approval, and AI-related/code-style guidance. (+147 / -46 lines) Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 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 |
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@CONTRIBUTING.md`:
- Line 51: Update the capitalization of the platform name in CONTRIBUTING.md by
changing the lowercase word "github" to the correct "GitHub" where it appears in
the sentence "It has many subtle and unexpected consequences on our github
repository." so the official branding is used.
🧹 Nitpick comments (3)
CONTRIBUTING.md (3)
54-54: Remove blank line inside blockquote.Markdown style guidelines recommend avoiding blank lines within blockquote blocks for consistency.
📝 Proposed fix
> So, pretty please, do not force-push. - > [!TIP]
69-71: Specify language for code fence.Markdown linters recommend specifying a language identifier for fenced code blocks to improve rendering and accessibility.
📝 Proposed fix
-> ``` +> ```text > Fix naming per `@Aircoookie`'s suggestion > ```
121-121: Consider a more descriptive section heading.The lowercase "more" heading is less formal than other section titles. Consider using a more descriptive title like "Additional Resources" for consistency.
📝 Proposed alternative
-### more +### Additional Resources You can find a collection of very useful tips and tricks here: https://github.com/wled-dev/WLED/wiki/How-to-properly-submit-a-PR
official style is "GitHub" Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@CONTRIBUTING.md`:
- Around line 63-66: Update the review guidance bullet items to use a more
polite tone by inserting "please" where appropriate; specifically edit the lines
"**Add new commits** - don't amend or force-push", "**Reply in the PR** - let us
know when you've addressed comments", "**Ask questions** - if something's
unclear, just ask!" and "**Be patient** - we're all volunteers here 😊" to
include "please" (e.g., "**Please add new commits** - don't amend or
force-push", "**Please reply in the PR** - let us know when you've addressed
comments", etc.), keeping the original meaning and punctuation consistent.
- Around line 68-71: The fenced code block containing the example commit message
"Fix naming per `@Aircoookie`'s suggestion" is missing a language identifier and
triggers markdownlint; update that fenced block in CONTRIBUTING.md by adding a
language token (for example `text`) after the opening backticks so the block
becomes ```text ... ``` to satisfy the linter.
DedeHai
left a comment
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.
looks good
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@CONTRIBUTING.md`:
- Line 120: Update the header string "### Additional Ressources" to use the
correct spelling "### Additional Resources" so the CONTRIBUTING.md heading reads
properly; locate the header line and replace "Ressources" with "Resources".
- Line 53: Remove the stray blank line inside the blockquote by deleting the
empty line that begins with ">" (the blank line within the quoted section) so
the blockquote lines are contiguous; simply ensure every quoted paragraph in the
blockquote starts with ">" with no intervening empty ">" line to satisfy
markdownlint.
🧹 Nitpick comments (1)
CONTRIBUTING.md (1)
62-65: Consider adding "please" for tone consistency.Line 62 uses "please" but lines 63-65 don't, creating slight inconsistency. Adding "please" to the other bullet points would create a more uniformly polite tone throughout the guidance.
✏️ Suggested refinement
- **Add new commits** - please don't amend or force-push -- **Reply in the PR** - let us know when you've addressed comments -- **Ask questions** - if something's unclear, just ask! -- **Be patient** - we're all volunteers here 😊 +- **Reply in the PR** - please let us know when you've addressed comments +- **Ask questions** - please ask if something's unclear! +- **Be patient** - please remember we're all volunteers here 😊
addressing minor recommendations by the rabbit
|
@coderabbitai as we are already updating the contributor's guide: please review the complete file, and summarize your recommendations also for unchanged parts of CONTRIBUTING.md. Our objective is to provide a guide that is clear and easy to follow, addresses the most important points, while being written in an inviting and casual tone. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 8257
✅ Strengths (What's Working Well)New sections are excellent:
Good existing content:
📝 Recommendations for Improvement1. Title & IntroductionCurrent: "Thank you for making WLED better!" Suggestion: Add a brief sentence about what WLED is for newcomers: # Thank you for making WLED better!
WLED is a community-driven project, and we appreciate every contribution! Here are a few suggestions to make it easier for you to contribute.2. "Describing your PR" SectionCurrent: Good content but feels slightly formal in places. Suggestions:
3. "Additional Resources" SectionCurrent: Single link, feels tacked on. Suggestions:
4. "Source Code from an AI agent or bot" SectionCurrent: Good warnings but slightly wordy and repetitive. Suggestions for streamlining: ## Using AI-Generated Code
> [!IMPORTANT]
> AI tools can be helpful, but as the contributor, **you're responsible for the code**.
Here's what we expect:
✅ **Understand it** - Don't just copy-paste because it "seems to work"
✅ **Review carefully** - AI can lose comments, introduce bugs, or make unnecessary changes
✅ **Be transparent** - Add a comment like `// This section was AI-generated` for larger chunks
✅ **Use AI for translation** - AI is great for translating comments to English (but verify technical terms!)
Remember: AI tools are powerful but "often wrong" - your judgment is essential! 😊5. Code Style SectionCurrent: Clear with good examples, but could be more inviting. Suggestions:
6. Comments SubsectionIssue: States "This is a recent change, the majority of the codebase still has comments without spaces." Suggestion: This might confuse contributors. Either:
7. Structure & FlowCurrent order works well, but consider these minor adjustments:
8. Missing Content (Optional Additions)Consider adding brief sections on:
9. Consistency & ToneMinor inconsistencies to address:
10. Typos & GrammarFound one minor issue:
🎯 Priority RecommendationsIf you want to make quick improvements with high impact:
Overall AssessmentThe file is very good overall! The new sections (Testing, Responding to Reviews, Merge Conflicts) are excellent additions. The force-push guidance is now much clearer. Main areas for improvement are:
The file successfully balances being informative with being approachable, which is exactly what you're aiming for! 🎉 Would you like me to generate a complete rewrite with these suggestions incorporated, or would you prefer to cherry-pick specific improvements? |
@coderabbitai I need some writing help. I think the intro should also mention why maintainers review contributions at all. Please find a wording to express this, while matching the overall style and tone of the guide. |
|
Tip For best results, initiate chat on the files or code changes.
Option 1: Direct & Friendly# Thank you for making WLED better!
WLED is a community-driven project, and we appreciate every contribution! We're here to help you improve your code and make it shine. ✨
Our maintainers review contributions to keep WLED efficient, maintainable, and focused. We're not just gatekeepers - we're collaborators who want your feature to succeed! We'll help refine your code, suggest improvements, and sometimes push back on changes that might add bloat or create maintenance headaches down the road. Think of us as guardians of WLED's architecture, working with you to make your contribution the best it can be.
Here are a few suggestions to make it easier for you to contribute:Option 2: More Concise & Casual# Thank you for making WLED better!
WLED is a community-driven project, and we love contributions! 🎉
Our maintainers are here for two things: **helping you** improve your code, and **keeping WLED** lean, efficient, and maintainable. We'll work with you to refine your contribution, but we'll also push back if something might create technical debt or add features without clear value. Don't take it personally - we're just protecting WLED's architecture while helping your contribution succeed!
Here are a few suggestions to make it easier for you to contribute:Option 3: Balanced & Welcoming# Thank you for making WLED better!
WLED is a community-driven project, and every contribution matters! We appreciate your time and effort.
**What maintainers do:** We're here to collaborate with you! We'll help improve your code, suggest better approaches, and ensure everything fits WLED's architecture. We're also guardians - we'll respectfully decline changes that might bloat the codebase or create maintenance issues. Our goal isn't to be picky; it's to keep WLED efficient and sustainable for everyone. 😊
Here are a few suggestions to make it easier for you to contribute:📍 Best PlacementI recommend putting this right at the top, as the introduction (replacing or expanding the current simple thank-you). This way:
Alternatively, you could add a separate "What to Expect from Reviews" section after "Describing your PR" and before "Testing Your Changes", but I think the intro placement is stronger - it sets the right expectations immediately. My PreferenceOption 2 feels the most natural and matches your existing casual tone ("pretty please", emojis). It's honest without being discouraging, and the "Don't take it personally" bit preemptively addresses potential friction. What do you think? Want me to adjust any of these? 😊 |
* extended opening section * section groups: Getting Started, During Review, After Approval, Coding Guidelines * removed duplications in AI section * improve friendly style * minor corrections
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.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CONTRIBUTING.md (1)
201-205:⚠️ Potential issue | 🟡 MinorGrammar cleanup in spacing rules.
Multiple small fixes in the same rule block.
✏️ Proposed fix
-Within the condition, no space should be between the parenthesis and variables. -Spaces between variables and operators are up to the authors discretion. -There should be no space between function names and their argument parenthesis. +Within the condition, no space should be between the parentheses and variables. +Spaces between variables and operators are up to the author's discretion. +There should be no space between function names and their argument parentheses.
🤖 Fix all issues with AI agents
In `@CONTRIBUTING.md`:
- Line 155: The sentence in CONTRIBUTING.md has a subject-verb agreement error:
change the phrase "the results is correct" (or "if the results is correct") to
use the plural verb "are" so it reads "the results are correct" (i.e., update
"check if the results is correct" to "check if the results are correct").
- Line 152: Replace the unhyphenated phrase "AI generated code" with the
hyphenated compound adjective "AI-generated code" in the CONTRIBUTING.md content
(look for the exact string "AI generated code") to correct the grammar.
- Line 174: The sentence "We use tabs for Indentation in Web files
(.html/.css/.js) and spaces (2 per indentation level) for all other files." uses
an uppercase "Indentation" in running text; change "Indentation" to lowercase
"indentation" so the sentence reads "We use tabs for indentation in Web files
(.html/.css/.js) and spaces (2 per indentation level) for all other files." in
CONTRIBUTING.md.
- Line 19: Update the sentence "When you chose to 'Commit changes', GitHub will
automatically create a PR from your fork." by correcting the typo: replace
"chose" with "choose" so it reads "When you choose to 'Commit changes', GitHub
will automatically create a PR from your fork."; ensure only that single word is
changed and punctuation/quoting remains unchanged.
- Line 64: The sentence in CONTRIBUTING.md uses past tense ("regularly lost")
but should be present tense; edit the sentence "For example, we regularly lost
review comments when the PR author force-pushes code changes. Our review bot
(coderabbit) may become unable to properly track changes, it gets confused or
stops responding to questions." to replace "regularly lost" with "regularly
lose" so it reads "For example, we regularly lose review comments when the PR
author force-pushes code changes..." ensuring tense agreement across the
sentence.
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@CONTRIBUTING.md`:
- Around line 67-68: Change the TIP sentence so it starts with a capital letter:
locate the markdown TIP block that begins with "[!TIP]" containing the sentence
"use [cherry-picking](...)" and update the word "use" to "Use" to match
capitalization consistency with other headings and sentences.
- Around line 154-162: Edit the sentences in the CONTRIBUTING.md section to fix
grammar and hyphenation: change “AI are still "Often-Wrong" ;-)” to use singular
verb and normal capitalization (e.g., “AI is still often wrong.”) and hyphenate
“AI generated” to “AI-generated” wherever it appears (for example in the header
“Best Practice with AI” and the bullet “Understand the code”/“Review carefully”
lines) so the text reads grammatically correct and consistently styled.
|
OK, I'm happy with the result now. |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@CONTRIBUTING.md`:
- Around line 17-19: The blockquote lines starting with "> " have multiple
spaces after the ">" which triggers markdownlint MD027; update the tip
blockquote by replacing each leading "> " with a single "> " so the three
lines ("**The easiest way to start your first PR**", "When viewing a file in
`wled/WLED`, click on the "pen" icon and start making changes.", "When you
choose to 'Commit changes', GitHub will automatically create a PR from your
fork.") use "> " (one space) after the ">" to satisfy MD027.
netmindz
left a comment
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.
Thanks for sorting that. Some good improvement in there
based on AI proposals in #5348
Summary by CodeRabbit