Skip to content

sync c++ and generic instructions with upstream#359

Open
softhack007 wants to merge 2 commits intomdevfrom
ai_instructions_update
Open

sync c++ and generic instructions with upstream#359
softhack007 wants to merge 2 commits intomdevfrom
ai_instructions_update

Conversation

@softhack007
Copy link
Copy Markdown
Collaborator

@softhack007 softhack007 commented Apr 14, 2026

updated instructions / coding guides, to incorporate changes from upstream WLED.

Summary by CodeRabbit

  • Documentation
    • Updated C++ coding conventions for contributors, including enum naming standards and const/constexpr guidelines.
    • Refined code review and contribution standards to strengthen feature-flag validation and approval processes.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 14, 2026

Warning

Rate limit exceeded

@softhack007 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 50 minutes and 5 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 50 minutes and 5 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 27736036-e255-45d3-86f9-a4ba930d4bbf

📥 Commits

Reviewing files that changed from the base of the PR and between 0433605 and fba4c4b.

📒 Files selected for processing (1)
  • docs/cpp.instructions.md

Walkthrough

Updates documentation for AI-assisted code review and C++ coding standards. Modifies AI-generated code attribution format, adds feature-flag name validation rules, expands const/constexpr guidance, updates bit-shift operation guidance for ESP8266, and introduces human-only instruction blocks.

Changes

Cohort / File(s) Summary
Copilot AI Review Guidance
.github/copilot-instructions.md
Refines AI review instructions by removing practice summarization requirement, adding strict validation for WLED_ENABLE_* and WLED_DISABLE_* feature-flag names against an allowlist, updating AI-generated code marking format to use block delimiters, tightening platformio.ini modification approval requirements, and adding FMEA process guidance for reliability/safety debates.
C++ Coding Standards
docs/cpp.instructions.md
Expands C++ conventions with PascalCase requirement for enum values, adds general style guidance (static helpers, magic number avoidance, include patterns), refines const/constexpr rules with read-only parameter guidance and explicit restriction on reference data members, clarifies bit-shift operations for ESP8266 targets, adds LittleFS filename caveat, updates AI-generated code marker format, and wraps new/edited sections with human-only instruction blocks.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • netmindz
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'sync c++ and generic instructions with upstream' directly and accurately describes the main changes—updates to copilot-instructions.md and cpp.instructions.md to align with upstream guidance.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (1)
.github/copilot-instructions.md (1)

97-99: Add a HUMAN_ONLY reference example for the new feature-flag allowlist rule.

The new AI-facing verification rule is good, but adding a short HUMAN_ONLY example (e.g., typo → corrected flag) would keep reference material aligned for contributors.

As per coding guidelines: “If new AI-facing rules were added without updating a related HUMAN_ONLY reference section, note this as a suggestion (not a required fix).”

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/copilot-instructions.md around lines 97 - 99, Add a HUMAN_ONLY
reference example for the new AI-facing feature-flag allowlist rule by inserting
a short example that shows a common typo and the corrected flag (e.g., "typo:
WLED_IR_DISABLE → corrected: WLED_DISABLE_INFRARED") so human reviewers see how
to map mistakes to the canonical names; reference the existing feature flag
tokens (WLED_ENABLE_*, WLED_DISABLE_* and specific names like INFRARED, AOTA,
MQTT) and place the example alongside the other HUMAN_ONLY examples for
consistency with the new rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/cpp.instructions.md`:
- Around line 174-175: The sentence "Use `const &` (or `&`)  inside loops - This
avoids constructing temporary objects on every access." has an extra space after
the closing backtick; remove the duplicate space so it reads "Use `const &` (or
`&`) inside loops - This avoids constructing temporary objects on every access."
to fix the spacing/wording issue.
- Around line 457-460: Update the heading "Bit Shifts Over Division (mainly for
RISC-V boards)" to reflect that ESP8266 is also affected—e.g., change it to "Bit
Shifts Over Division (RISC-V targets and ESP8266)" or "Bit Shifts Over Division
(RISC-V and ESP8266)"; ensure the new heading aligns with the paragraph text
that mentions ESP32/ESP32-S3 and RISC-V targets (ESP32-C3/C6/P4) and ESP8266 so
messaging is consistent.

---

Nitpick comments:
In @.github/copilot-instructions.md:
- Around line 97-99: Add a HUMAN_ONLY reference example for the new AI-facing
feature-flag allowlist rule by inserting a short example that shows a common
typo and the corrected flag (e.g., "typo: WLED_IR_DISABLE → corrected:
WLED_DISABLE_INFRARED") so human reviewers see how to map mistakes to the
canonical names; reference the existing feature flag tokens (WLED_ENABLE_*,
WLED_DISABLE_* and specific names like INFRARED, AOTA, MQTT) and place the
example alongside the other HUMAN_ONLY examples for consistency with the new
rule.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 87a2b556-9681-4ce3-aa4c-70851a64fcb3

📥 Commits

Reviewing files that changed from the base of the PR and between 36241e8 and 0433605.

📒 Files selected for processing (2)
  • .github/copilot-instructions.md
  • docs/cpp.instructions.md

@softhack007
Copy link
Copy Markdown
Collaborator Author

@coderabbitai please check if <!-- HUMAN_ONLY_START --> / <!-- HUMAN_ONLY_END --> tags are still consistent (no overlap, every START is followed by END)

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 14, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all files containing HUMAN_ONLY tags
echo "=== Files with HUMAN_ONLY tags ==="
rg -l 'HUMAN_ONLY' 

echo ""
echo "=== All HUMAN_ONLY tag occurrences with line numbers ==="
rg -n 'HUMAN_ONLY' --no-heading

Length of output: 6099


@softhack007: All <!-- HUMAN_ONLY_START --> / <!-- HUMAN_ONLY_END --> tags are consistent across all three files that contain them. Here's the summary:

File START count END count Issues
docs/cpp.instructions.md 22 22 ✅ None
docs/cicd.instructions.md 1 1 ✅ None
docs/esp-idf.instructions.md 27 27 ✅ None

Findings:

  • Every START has a matching END — no unclosed blocks.
  • No END appears without a preceding START.
  • No nesting or overlapping — a new START never appears before the previous block's END.
  • There are a few back-to-back adjacent blocks (e.g., docs/cpp.instructions.md lines 392/393, and docs/esp-idf.instructions.md lines 427/429 and 599/600), where an END is immediately followed by a START on the next line — these are consecutive blocks, not overlaps, and are valid.

🧠 Learnings used
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2026-04-04T14:32:02.618Z
Learning: In MoonModules/WLED-MM, clang-format is intentionally NOT enforced via CI or .clang-format config because it would cause merge conflicts when cherry-picking features from upstream WLED (which uses different formatting). Instead, "soft" formatting rules are documented in .github/cpp.instructions.md. Do not recommend adding clang-format to CI as a follow-up to missing automated enforcement — the decision is deliberate.

Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2026-04-04T00:29:49.127Z
Learning: In MoonModules/WLED-MM, function-level doc comment blocks (e.g. /* ***** */ style headers) are NOT required during code reviews, especially not for previously existing functions. Do not flag missing function-level comments as review issues in this repository.

Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2026-04-07T16:49:44.284Z
Learning: In MoonModules/WLED-MM, `.github/agent-build.instructions.md` is intentionally kept in `.github/` (not moved to `docs/`) because it is exclusively meant for GitHub Copilot agent tasks, not for CodeRabbit code reviews. It should NOT be referenced in `.coderabbit.yaml` and should not be flagged as misplaced or missing from `docs/`.

Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: wled00/fcn_declare.h:403-406
Timestamp: 2025-10-22T21:59:24.331Z
Learning: In MoonModules/WLED-MM PR `#270`, the out-of-bounds null-terminator write in wled00/util.cpp (extractModeSlider) is deferred and tracked in Issue `#272`; do not address it within PR `#270`.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant