-
Notifications
You must be signed in to change notification settings - Fork 158
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
Updated coderabbit rules #974
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for docs-optimism ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughThe changes in this pull request primarily involve updates to the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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: 0
🧹 Outside diff range and nitpick comments (2)
.coderabbit.yaml (2)
19-22
: Excellent guidelines for header capitalization.These rules effectively address the PR objectives regarding the capitalization of proper nouns in headers. They provide clear instructions for using sentence case while preserving the capitalization of proper nouns, technical terms, and acronyms. The examples given (Optimism, OP Mainnet, Ethereum, JavaScript, NFT) are particularly helpful.
Consider adding a note about consistency in capitalization across the document for the same terms, to ensure that "JavaScript" isn't written as "Javascript" in some places, for example.
24-24
: Good practice to reference a predefined list of terms.Referring to a predefined list of proper nouns and technical terms is an excellent way to ensure consistency across the project documentation. This aligns well with the PR objectives of preserving proper noun capitalization.
Consider including the actual list of terms in this configuration file, or provide a clear reference to where this list can be found. This will make it easier for contributors to access and maintain the list.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- .coderabbit.yaml (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
.coderabbit.yaml (2)
17-17
: LGTM: Clear distinction between title case and sentence case usage.This guideline provides clear instructions on when to use title case (for buttons, tab names, page names, and links) and when to use sentence case (for body content and short phrases). This distinction helps maintain consistency throughout the documentation.
23-23
: Good addition of a manual review mechanism.This instruction to flag headers that inconsistently apply the rules for manual review is a valuable safeguard. It aligns well with the PR objectives, particularly the mention of manual flagging for edge cases.
Could you please clarify how this flagging mechanism will be implemented? Who will be responsible for performing the manual review of flagged headers? Consider adding more details to ensure this process is clear and actionable.
- For H1, H2, and H3 headers: | ||
1. Use sentence case, capitalizing only the first word. | ||
2. Preserve the capitalization of proper nouns, technical terms, and acronyms as defined in the 'nouns.txt' file located in the root directory of the project. | ||
3. Do not automatically lowercase words that appear in the 'nouns.txt' file, regardless of their position in the header. |
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.
I'm slightly confused here. So this is saying that CodeRAbbit should look at nouns.txt
as a whitelist of sorts for words that are allowed to be capitalized in headers correct?
If so, I think we need to take another pass at the list. I think it should exclusively be proper nouns and not just web3 terms
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.
Yeah, so this list is by no means a standard, we can always update it, what we have there are placeholders.
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.
agree with both sentiments here,
- seems like we get the rule and the nouns.txt file in place and we can add to it as we go along (just like we did/have been doing with the words.txt file).
- but also wonder if we could have coderabbit do an initial sweep thru the docs to populate the current list more?
Co-authored-by: Bradley Camacho <[email protected]>
Co-authored-by: Bradley Camacho <[email protected]>
DAO | ||
EVM | ||
L2 | ||
dApp |
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.
@krofax we no longer use dApp
here at OP Labs, a decision from product team (before you two joined). all instances were removed from docs a few months ago and replaced with just app
seems like we could add to this list: OP Stack, OP Chains, Superchain, Collective, Foundation, Security Council, Alt-DA, Mission Grants, Supersim, SuperchainERC20, CLI, Faucet, Dev Console, Paymaster, Tokenlist
Description
Updated CodeRabbit rules to:
Proper Noun Preservation: Proper nouns like "Optimism", "OP Mainnet", and "Ethereum" will now retain their capitalization in headers (H1, H2, H3), even with sentence case.
Manual Flagging for Edge Cases: Any headers that don’t follow these rules exactly will be flagged for manual review to avoid false positives.
Tests
Additional context
Metadata