Library conventions - #689
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe PR introduces comprehensive machine-readable conventions, procedures, and documentation to standardize external contributions and internal release workflows for the Stellar Contracts repository, including code quality audit guidance, release preparation steps, development conventions, and contribution policies. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
CLAUDE.md (2)
8-23: Add language identifier to code block.The code block should specify a language for proper syntax highlighting and markdown compliance. Use
textfor the directory tree structure.📝 Suggested fix
-``` +```text packages/ ├── access/ # access_control, ownable, role_transfer🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` around lines 8 - 23, The fenced code block containing the directory tree should include a language identifier for proper highlighting and markdown compliance; change the opening fence from ``` to ```text so the block starting with "packages/" and the tree lines (e.g., "├── access/ ...", "└── zk-email/ ...") becomes ```text ... ```; ensure you update the single opening fence only and leave the directory contents unchanged.
27-32: Add language identifier to code block.The code block should specify a language identifier. Use
textfor the directory structure.📝 Suggested fix
-``` +```text <package>/src/<module>/ ├── mod.rs # docstring, trait (#[contracttrait]), errors, constants, events🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` around lines 27 - 32, The markdown code block in CLAUDE.md that shows the directory tree for "<package>/src/<module>/" is missing a language identifier; edit CLAUDE.md and change the fenced block to use a language tag (use "text") by adding ```text before the tree and keeping the closing ``` after, so the block that contains "<package>/src/<module>/", "mod.rs", "storage.rs", and "test.rs" is fenced as ```text ... ``` to satisfy the linter..claude/skills/code-quality.md (3)
297-302: Add language identifier to code block.The code block should specify a language for proper rendering. Use
markdownortextfor the documentation example.📝 Suggested fix
- ``` + ```markdown /// # Events ///🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/code-quality.md around lines 297 - 302, The fenced code block containing the documentation example starting with "/// # Events" is missing a language identifier; update the opening fence (the triple backticks before the "/// # Events" block) to include a language such as markdown or text (e.g., change ``` to ```markdown) so the example renders correctly in docs and syntax-highlighters; target the fenced block around the "/// # Events" comment in the diff.
360-382: Spelling inconsistency: authorization vs authorisation.The section header uses "authorization" (American spelling) while line 367 uses "authorisation" (British spelling). Use consistent spelling throughout the document.
✏️ Suggested fix
siblings do neither — they accept a `caller: &Address` purely for event emission. The split must be honoured: `require_auth()` inside a - `_no_auth` function, or omitting it from the high-level entry point, is + `_no_auth` function, or omitting it from the high-level entry point, is a violation. - **Never call `require_auth()` twice on the same address inside oneOr update the section header to match:
-### Functions and authorization +### Functions and authorisationChoose one spelling and apply it consistently.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/code-quality.md around lines 360 - 382, The document uses mixed spellings "authorization" and "authorisation"; pick one and make it consistent (e.g., replace all occurrences of "authorisation" with "authorization") across this file, including the section header "Functions and authorization" and the sentence mentioning "borrowed `&Address` arguments" and `require_auth()` so every instance uses the same spelling.
148-148: Add language identifier to code block.The code block should specify a language for proper rendering. Use
textfor the directory structure.📝 Suggested fix
-``` +```text <module>/ ├── mod.rs # trait, errors, constants, events, public re-exports🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/code-quality.md at line 148, Update the opening fenced code block so it specifies the language identifier `text` (change the leading ``` to ```text) for the directory-structure snippet (the block that starts with the triple backticks and contains "<module>/ ├── mod.rs ...") to ensure correct rendering..claude/skills/release-prep.md (1)
37-37: Line number approximation is slightly off.The comment states
[workspace.package] version (line ~52), but based on the actualCargo.toml, this section appears around line 44. While the~indicates an approximation, consider updating to(line ~44)for better accuracy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/release-prep.md at line 37, Update the approximate line number in the review comment so it matches the Cargo.toml location: change the note that says `[workspace.package] version (line ~52)` to `[workspace.package] version (line ~44)` (or similar ~44) so the reference to the `[workspace.package]` section is accurate; edit the comment text that mentions `[workspace.package]` to reflect the corrected approximate line number.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/skills/code-quality.md:
- Around line 435-441: The documentation contains a duplicated paragraph about
traits and contract types (the block starting "The methods that are closely
relevant..." which repeats the guidance about placing methods under a trait and
providing default implementations); remove the duplicate instance in the
Documentation section so only the original paragraph under "Traits and contract
types" remains, ensuring the remaining text still advises implementing trait
methods on the contract and providing default implementations that use storage
functions.
In @.claude/skills/release-prep.md:
- Line 51: The command includes a macOS-specific PATH export string
("/opt/homebrew/bin:$HOME/.cargo/bin:$PATH"); either remove the
platform-specific prefix and use the platform-agnostic "cargo build" command, or
keep the export but add a short note that the PATH modification is optional and
only needed on macOS/Apple Silicon (explain why the extra PATH entry is required
if there is a specific tool dependency). Update the line containing the PATH
export accordingly and ensure the doc clarifies the platform scope.
---
Nitpick comments:
In @.claude/skills/code-quality.md:
- Around line 297-302: The fenced code block containing the documentation
example starting with "/// # Events" is missing a language identifier; update
the opening fence (the triple backticks before the "/// # Events" block) to
include a language such as markdown or text (e.g., change ``` to ```markdown) so
the example renders correctly in docs and syntax-highlighters; target the fenced
block around the "/// # Events" comment in the diff.
- Around line 360-382: The document uses mixed spellings "authorization" and
"authorisation"; pick one and make it consistent (e.g., replace all occurrences
of "authorisation" with "authorization") across this file, including the section
header "Functions and authorization" and the sentence mentioning "borrowed
`&Address` arguments" and `require_auth()` so every instance uses the same
spelling.
- Line 148: Update the opening fenced code block so it specifies the language
identifier `text` (change the leading ``` to ```text) for the
directory-structure snippet (the block that starts with the triple backticks and
contains "<module>/ ├── mod.rs ...") to ensure correct rendering.
In @.claude/skills/release-prep.md:
- Line 37: Update the approximate line number in the review comment so it
matches the Cargo.toml location: change the note that says `[workspace.package]
version (line ~52)` to `[workspace.package] version (line ~44)` (or similar ~44)
so the reference to the `[workspace.package]` section is accurate; edit the
comment text that mentions `[workspace.package]` to reflect the corrected
approximate line number.
In `@CLAUDE.md`:
- Around line 8-23: The fenced code block containing the directory tree should
include a language identifier for proper highlighting and markdown compliance;
change the opening fence from ``` to ```text so the block starting with
"packages/" and the tree lines (e.g., "├── access/ ...", "└── zk-email/ ...")
becomes ```text ... ```; ensure you update the single opening fence only and
leave the directory contents unchanged.
- Around line 27-32: The markdown code block in CLAUDE.md that shows the
directory tree for "<package>/src/<module>/" is missing a language identifier;
edit CLAUDE.md and change the fenced block to use a language tag (use "text") by
adding ```text before the tree and keeping the closing ``` after, so the block
that contains "<package>/src/<module>/", "mod.rs", "storage.rs", and "test.rs"
is fenced as ```text ... ``` to satisfy the linter.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 32410266-9888-4e69-ae34-281e4e1346ed
📒 Files selected for processing (6)
.claude/skills/code-quality.md.claude/skills/release-prep.md.gitignoreCLAUDE.mdCONTRIBUTING.mdpackages/tokens/src/fungible/test.rs
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #689 +/- ##
=======================================
Coverage 96.24% 96.24%
=======================================
Files 59 59
Lines 6206 6206
=======================================
Hits 5973 5973
Misses 233 233 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fixes #688
PR Checklist
Summary by CodeRabbit
Documentation
Chores