Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughUpdate documentation and CI/config: clarify pytest async-test handling, relocate frontend guide links into Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the code review guidelines to clarify that @pytest.mark.asyncio is not required and corrects the paths for frontend documentation. Feedback identifies a missing config.yaml file mentioned in the PR description and notes inconsistent indentation in the updated list items.
| - `./frontend_design_guide.md` | ||
| - `./frontend_controls.md` | ||
| - `.agents/frontend_design_guide.md` | ||
| - `.agents/frontend_controls.md` |
There was a problem hiding this comment.
The bot suggested the path should be from the root. Not sure - @tawnymanticore ?
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.gemini/styleguide.md (1)
9-10: Polish spelling/grammar to reduce review noise.There are a few doc typos (“self explanitory”, “non obvious”, “where ever”, “modifing”, “a SDK”, “a input”, missing period after
etc.). This is low risk but worth cleaning up in a style guide.Also applies to: 16-18, 27-27, 42-45
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gemini/styleguide.md around lines 9 - 10, The style guide contains several spelling/grammar typos that should be corrected to reduce review noise; update the listed phrases ("self explanitory" -> "self-explanatory", "non obvious" -> "non-obvious", "where ever" -> "wherever", "modifing" -> "modifying", "a SDK" -> "an SDK", "a input" -> "an input") and add the missing period after "etc." and any similar punctuation errors, and sweep the document to fix the same issues where noted (other occurrences of these phrases in the file).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gemini/styleguide.md:
- Line 3: Change the third-line heading "### Issues to watch for" to an H2 by
replacing the triple-hash with double-hash (i.e., "## Issues to watch for") so
the document's heading sequence increments from H1 to H2 and resolves the
markdownlint MD001 warning; update the heading text in .gemini/styleguide.md
where the "### Issues to watch for" heading appears.
- Line 13: The file contains the literal token "`TODO`" (and mentions "`FIXME`")
which triggers the CI Debug Detector and blocks merging; update the offending
sentence that mandates "TODO comments" so it does not include the exact
substring TODO/FIXME (e.g., reword to "comments requiring follow-up" or "items
to resolve before final PR") and ensure the phrase in the
`.gemini/styleguide.md` that currently reads "`TODO` comments: before the final
PR, all `TODO` comments must be resolved..." is rewritten to avoid those exact
tokens while preserving the intent that outstanding work must be resolved before
final PR.
---
Nitpick comments:
In @.gemini/styleguide.md:
- Around line 9-10: The style guide contains several spelling/grammar typos that
should be corrected to reduce review noise; update the listed phrases ("self
explanitory" -> "self-explanatory", "non obvious" -> "non-obvious", "where ever"
-> "wherever", "modifing" -> "modifying", "a SDK" -> "an SDK", "a input" -> "an
input") and add the missing period after "etc." and any similar punctuation
errors, and sweep the document to fix the same issues where noted (other
occurrences of these phrases in the file).
🪄 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: a80bf231-a7a8-4085-bb04-d4c96d1d96cf
📒 Files selected for processing (3)
.agents/code_review_guidelines.md.gemini/config.yaml.gemini/styleguide.md
📊 Coverage ReportOverall Coverage: 92% Diff: origin/main...HEADNo lines with coverage information in this diff.
|
| @@ -0,0 +1,47 @@ | |||
| # Kiln Code Review Guidelines | |||
There was a problem hiding this comment.
can we do this without duplication of .agents/code_review_guidelines.md? It'll get out of sync if we duplicate. I've been trying to migrate to 1 source of truth and scripts like setup_claude.sh
Maybe even just "Read the core review guidelines at ...."
There was a problem hiding this comment.
True, did not really think of that but I assume it can indeed go read the other files. Just updated, we will see if it does
| @@ -0,0 +1,5 @@ | |||
| ignore_patterns: | |||
There was a problem hiding this comment.
hmmmm. I have another PR to stop adding more top level cruft. Will is respect any other DIR? .agents/.gemini? Configurable?
There was a problem hiding this comment.
I can't find a way. Still think change is worth it, gemini CR is great. But make the one a pointer so no dupe??
There was a problem hiding this comment.
Did not see a way in the docs but also did not look that much; and if no way, probably not worth building our own layer on top to spawn the configs. Worst case if it messes up we find out quickly and can fix easily.
The main intent with this one was to get it to stop dropping billions of comments about needing @pytest.mark.asyncio on all the test cases
2 participants
What does this PR do?
Add styleguide for Gemini code review bot; and config.yaml:
Also added a note to prevent .agents/ from complaining about
@pytest.mark.asynciodecorator missing on async tests, since this is optional.Checklists
Summary by CodeRabbit