doc: update style guide #10424
doc: update style guide #10424
Conversation
WalkthroughWalkthroughThe recent update enriches the Changes
Recent Review DetailsConfiguration used: .coderabbit.yml Files selected for processing (1)
Additional Context UsedLanguageTool (16)
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (7)
packages/contracts-bedrock/STYLE_GUIDE.md (7)
21-21: Consider using a hyphen in "Low Level Calls" to form the compound adjective "Low-Level Calls".
Line range hint
51-51: Add an apostrophe to indicate possession in "contracts semver" to "contract's semver".- @custom:semver: Add to `version` variable which indicate the contracts semver. + @custom:semver: Add to `version` variable which indicate the contract's semver.
Line range hint
79-81: Clarify the use of "should" in the description of immutable variables. Consider using more assertive language to enhance the directive nature of the guidelines.- should be in `SCREAMING_SNAKE_CASE` - should be `internal` - should have a hand written getter function + must be in `SCREAMING_SNAKE_CASE` + must be `internal` + must have a hand-written getter function
Line range hint
113-113: Consider adding a comma after "contract" for better readability.- All (non-library and non-abstract) contracts MUST extend the `Semver` base contract which + All (non-library and non-abstract) contracts MUST extend the `Semver` base contract, which
Line range hint
163-163: Consider adding a comma after "describe" for better readability.- Solidity `contract`s are used to organize the test suite similar to how mocha uses describe. + Solidity `contract`s are used to organize the test suite similar to how mocha uses describe,
Line range hint
193-193: Correct the abbreviation "ie." to "i.e." for proper formatting.- ie. `TargetContract_Getters_Test`. + i.e. `TargetContract_Getters_Test`.
Line range hint
150-150: Specify a language for the fenced code block to enable proper syntax highlighting.- ``` + ```typescript
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #10424 +/- ##
============================================
- Coverage 42.30% 29.22% -13.08%
============================================
Files 73 31 -42
Lines 4848 2898 -1950
Branches 766 614 -152
============================================
- Hits 2051 847 -1204
+ Misses 2686 1976 -710
+ Partials 111 75 -36
Flags with carried forward coverage won't be shown. Click here to find out more. |
Co-authored-by: Maurelian <john@oplabs.co>
There was a problem hiding this comment.
Actionable comments posted: 2
Out of diff range and nitpick comments (3)
packages/contracts-bedrock/STYLE_GUIDE.md (3)
Line range hint
153-153: Hyphenate "Low-Level" in the section title.Consider changing "Low Level Calls" to "Low-Level Calls" to form a correct compound adjective in the section title:
- [Expect Revert with Low-Level Calls](#expect-revert-with-low-level-calls)
Line range hint
192-192: Correct the abbreviation for "that is".Change "ie." to "i.e.," to correctly punctuate the abbreviation for "that is":
- `TargetContract_Getters_Test` for contracts containing grouped getter functions, i.e., `TargetContract_Getters_Test`.
Line range hint
79-81: Clarify the guidance on immutable variables.The current guidance on immutable variables could be clearer. Consider rephrasing to:
Immutable variables should: - Be in `SCREAMING_SNAKE_CASE` - Be `internal` - Have a manually written getter functionThis change enhances clarity and readability.
|
Semgrep found 1 Service 'op_stack_go_builder' is running with a writable root filesystem. This may allow malicious applications to download and run additional payloads, or modify container files. If an application inside a container has to save something temporarily consider using a tmpfs. Add 'read_only: true' to this service to prevent this. Ignore this finding from writable-filesystem-service.Semgrep found 1 Service 'op_stack_go_builder' allows for privilege escalation via setuid or setgid binaries. Add 'no-new-privileges:true' in 'security_opt' to prevent this. Ignore this finding from no-new-privileges. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
No description provided.