docs: code guidelines changes #20482
Conversation
WalkthroughWalkthroughThe recent changes enhance the code quality, versioning, and release processes. The Changes
Sequence Diagram(s) (Beta)No sequence diagrams are necessary for these documentation updates. 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
Outside diff range and nitpick comments (2)
RELEASES.md (2)
Line range hint
3-3: Add a comma after "confusing" for grammatical correctness.- While this is confusing lets break it down: 0ver is used for the main Cosmos-SDK dependency (`github.com/cosmos/cosmos-sdk`) and Semver is used for all other dependencies. + While this is confusing, lets break it down: 0ver is used for the main Cosmos-SDK dependency (`github.com/cosmos/cosmos-sdk`) and Semver is used for all other dependencies.
Line range hint
3-3: Add an apostrophe to "lets" to form the contraction for "let us".- While this is confusing lets break it down: 0ver is used for the main Cosmos-SDK dependency (`github.com/cosmos/cosmos-sdk`) and Semver is used for all other dependencies. + While this is confusing, let's break it down: 0ver is used for the main Cosmos-SDK dependency (`github.com/cosmos/cosmos-sdk`) and Semver is used for all other dependencies.
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (2)
docs/0ver.pngis excluded by!**/*.pngdocs/semver.pngis excluded by!**/*.png
Files selected for processing (3)
- CODING_GUIDELINES.md (4 hunks)
- RELEASES.md (1 hunks)
- RELEASE_PROCESS.md (2 hunks)
Additional Context Used
LanguageTool (37)
CODING_GUIDELINES.md (12)
Near line 7: This word is normally spelled with a hyphen.
Context: ...ments. ## API & Design * Code must be well structured: * packages must have a limited res...
Rule ID: EN_COMPOUNDS_WELL_STRUCTURED
Near line 15: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...sign patterns and naming conventions. * publicly-exposed functions are named logically, have for...
Rule ID: HYPHENATED_LY_ADVERB_ADJECTIVE
Near line 44: A comma may be missing after the conjunctive/linking adverb ‘Hence’.
Context: ...the real users and is ready for launch. Hence we often talk about **User Acceptance T...
Rule ID: SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA
Near line 55: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...xpecting as they test user scenarios. * Automated acceptance tests executed and passed on...
Rule ID: ENGLISH_WORD_REPEAT_BEGINNING_RULE
Near line 64: Possible missing comma found.
Context: ...tinfowler.com/bliki/GivenWhenThen.html) format where: * GIVEN: A set of initial c...
Rule ID: AI_HYDRA_LEO_MISSING_COMMA
Near line 106: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it.
Context: ...therwise. When testing a function under a variety of different inputs, we prefer to use [tab...
Rule ID: A_VARIETY_OF
Near line 107: The adjective “table-driven” is spelled with a hyphen.
Context: ... of different inputs, we prefer to use [table driven tests](https://github.com/golang/go/wik...
Rule ID: DRIVEN_HYPHEN
Near line 107: The adjective “Table-driven” is spelled with a hyphen.
Context: ...b.com/golang/go/wiki/TableDrivenTests). Table driven test error messages should follow the f...
Rule ID: DRIVEN_HYPHEN
Near line 151: The word “cross-check” is spelled with a hyphen.
Context: ...and functional testing. ### QA Team to cross check Acceptance Tests Once the AT are defin...
Rule ID: CROSS_COMPOUNDS
Near line 156: It seems that “to” is missing before the verb.
Context: ...date the user experience will be good * validate the implementation conforms the accepta...
Rule ID: MISSING_TO_BETWEEN_BE_AND_VB
Near line 156: Possible missing preposition found.
Context: ... * validate the implementation conforms the acceptance tests * by having a broader ...
Rule ID: AI_HYDRA_LEO_MISSING_TO
Near line 157: Possible missing article found.
Context: ...ng a broader overview of the use cases, QA team should be able to define **test su...
Rule ID: AI_HYDRA_LEO_MISSING_THERELEASES.md (2)
Near line 3: Possible missing comma found.
Context: ...er](https://semver.org/). While this is confusing lets break it down: 0ver is used for th...
Rule ID: AI_HYDRA_LEO_MISSING_COMMA
Near line 3: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...//semver.org/). While this is confusing lets break it down: 0ver is used for the mai...
Rule ID: AI_HYDRA_LEO_APOSTROPHE_S_XSRELEASE_PROCESS.md (23)
Near line 11: Possible missing comma found.
Context: ...)_section. Before making a new major release we do beta and release candidate releas...
Rule ID: AI_HYDRA_LEO_MISSING_COMMA
Near line 26: Possible typo. Did you mean “the” or “All”?
Context: ...branch prepare a new version section in theCHANGELOG.md* All links must point to their respective pu...
Rule ID: DT_PRP
Near line 29: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...d changes in the version. This is needed so the bot knows which entries to add to t...
Rule ID: COMMA_COMPOUND_SENTENCE_2
Near line 30: It appears that hyphens are missing in the adjective “up-to-date”.
Context: ... verify that theUPGRADING.mdfile is up to date and contains all the necessary informat...
Rule ID: UP_TO_DATE_HYPHEN
Near line 34: Loose punctuation mark.
Context: ...sted directly on main. *build.yml: Only the SDK and SimApp needs to be bui...
Rule ID: UNLIKELY_OPENING_PUNCTUATION
Near line 39: It appears that a comma is missing.
Context: ... in the release branch. * from this point we unfreeze main. * the SDK teams c...
Rule ID: DURING_THAT_TIME_COMMA
Near line 40: Consider a shorter alternative to avoid wordiness.
Context: ...orate and do their best to run testnets in order to validate the release. * when bugs a...
Rule ID: IN_ORDER_TO_PREMIUM
Near line 57: This word is normally spelled with a hyphen.
Context: ...kporting PRs (usually done by automatic cherry pick followed by a PRs to the release branch...
Rule ID: EN_COMPOUNDS_CHERRY_PICK
Near line 57: The plural noun “PRs” cannot be used with the article “a”. Did you mean “a PR” or “PRs”?
Context: ...ne by automatic cherry pick followed by a PRs to the release branch). The backports m...
Rule ID: A_NNS
Near line 69: Possible missing preposition found.
Context: ...release is a submodule update, first go the submodule folder and name the tag prepe...
Rule ID: AI_HYDRA_LEO_MISSING_TO
Near line 75: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ease) until they reach End Of Life. Major Release series is maintained in complia...
Rule ID: ENGLISH_WORD_REPEAT_BEGINNING_RULE
Near line 84: The hyphen in Widely-used is redundant.
Context: ...xes to releases that are not supported. Widely-used (decided at SDK team's discretion) unsu...
Rule ID: ADVERB_LY_HYPHEN_FIX
Near line 110: Did you mean ‘different from’? ‘Different than’ is often considered colloquial style.
Context: ...ements for stable changes are different than those that are candidates to be merged ...
Rule ID: DIFFERENT_THAN
Near line 111: This phrase might be unnecessarily wordy.
Context: .... When preparing future major releases, our aim is to design the most elegant, user-friend...
Rule ID: MY_HOPE_IS
Near line 125: Possible typo: you repeated a word
Context: ...e Release Update (SRU) * High-impact bugs * Bugs that may directly cause a security vuln...
Rule ID: ENGLISH_WORD_REPEAT_RULE
Near line 141: The plural noun “Changes” expects the verb form “introduce”.
Context: ...-updates-guidelines.md). * Changes that introduces API breakages (e.g. public functions an...
Rule ID: NNS_THAT_VBZ
Near line 153: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...se state corruption or data loss. * Bugs that may directly or indirectly cause a...
Rule ID: ENGLISH_WORD_REPEAT_BEGINNING_RULE
Near line 159: Possible missing article found.
Context: ...y included in stable point-releases As rule of thumb, the following changes will **...
Rule ID: AI_HYDRA_LEO_MISSING_A
Near line 174: The preposition ‘to’ seems more likely in this position.
Context: ... [Impact] An explanation of the bug on users and justification for backporting...
Rule ID: AI_HYDRA_LEO_REPLACE_ON_TO
Near line 209: A comma may be missing after the conjunctive/linking adverb ‘Currently’.
Context: ...t Release Procedure](CONTRIBUTING.md). Currently residing Stable Release Managers: * @t...
Rule ID: SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA
Near line 222: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ted in an app (e.gx/modules). * Modules that are not imported into an app and a...
Rule ID: ENGLISH_WORD_REPEAT_BEGINNING_RULE
Near line 223: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...andalone module (e.g.cosmovisor). 2. Modules that do not depend on the Cosmos SDK. ...
Rule ID: ENGLISH_WORD_REPEAT_BEGINNING_RULE
Near line 230: Possible missing preposition found.
Context: ...same time or soon after a major version Cosmos SDK itself. Those modules can be consid...
Rule ID: AI_HYDRA_LEO_MISSING_OF
Markdownlint (51)
CODING_GUIDELINES.md (17)
8: Expected: 2; Actual: 4
Unordered list indentation
9: Expected: 2; Actual: 4
Unordered list indentation
10: Expected: 2; Actual: 4
Unordered list indentation
28: Expected: 2; Actual: 4
Unordered list indentation
29: Expected: 2; Actual: 4
Unordered list indentation
30: Expected: 2; Actual: 4
Unordered list indentation
31: Expected: 2; Actual: 4
Unordered list indentation
136: Expected: 2; Actual: 4
Unordered list indentation
137: Expected: 2; Actual: 4
Unordered list indentation
140: Expected: 2; Actual: 4
Unordered list indentation
50: Expected: 1; Actual: 2
Multiple consecutive blank lines
76: null
Multiple spaces after blockquote symbol
77: null
Multiple spaces after blockquote symbol
79: null
Multiple spaces after blockquote symbol
81: null
Multiple spaces after blockquote symbol
82: null
Multiple spaces after blockquote symbol
83: null
Multiple spaces after blockquote symbolRELEASES.md (1)
7: Expected: 0 or 2; Actual: 1
Trailing spacesRELEASE_PROCESS.md (33)
18: Expected: 2; Actual: 4
Unordered list indentation
19: Expected: 2; Actual: 4
Unordered list indentation
20: Expected: 2; Actual: 4
Unordered list indentation
21: Expected: 2; Actual: 4
Unordered list indentation
22: Expected: 2; Actual: 4
Unordered list indentation
24: Expected: 2; Actual: 4
Unordered list indentation
25: Expected: 2; Actual: 4
Unordered list indentation
27: Expected: 2; Actual: 4
Unordered list indentation
28: Expected: 2; Actual: 4
Unordered list indentation
29: Expected: 2; Actual: 4
Unordered list indentation
30: Expected: 2; Actual: 4
Unordered list indentation
32: Expected: 2; Actual: 4
Unordered list indentation
33: Expected: 4; Actual: 8
Unordered list indentation
34: Expected: 2; Actual: 4
Unordered list indentation
35: Expected: 4; Actual: 8
Unordered list indentation
36: Expected: 4; Actual: 8
Unordered list indentation
37: Expected: 2; Actual: 4
Unordered list indentation
39: Expected: 2; Actual: 4
Unordered list indentation
40: Expected: 2; Actual: 4
Unordered list indentation
41: Expected: 2; Actual: 4
Unordered list indentation
42: Expected: 2; Actual: 4
Unordered list indentation
44: Expected: 2; Actual: 4
Unordered list indentation
45: Expected: 2; Actual: 4
Unordered list indentation
46: Expected: 2; Actual: 4
Unordered list indentation
47: Expected: 2; Actual: 4
Unordered list indentation
69: Expected: 2; Actual: 4
Unordered list indentation
126: Expected: 2; Actual: 4
Unordered list indentation
127: Expected: 2; Actual: 4
Unordered list indentation
129: Expected: 2; Actual: 4
Unordered list indentation
131: Expected: 2; Actual: 4
Unordered list indentation
132: Expected: 2; Actual: 4
Unordered list indentation
133: Expected: 2; Actual: 4
Unordered list indentation
135: Expected: 2; Actual: 4
Unordered list indentation
Path-based Instructions (3)
RELEASES.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"CODING_GUIDELINES.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"RELEASE_PROCESS.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Additional comments not posted (8)
RELEASES.md (2)
9-16: The Mermaid flowchart is a clear and effective way to illustrate the versioning decision process. Good addition!
22-29: The Mermaid flowchart for 0ver dependencies is well-integrated and provides a straightforward visual representation of the versioning process.CODING_GUIDELINES.md (4)
10-10: The addition of considerations for maintainability and testability in the API & Design section is valuable for guiding developers.
21-21: Highlighting the importance of avoiding unnecessary operations or memory allocations in the Performance section is crucial for efficient code.
Line range hint
25-34: The Security section effectively addresses critical areas such as transaction verification and thread safety. It's important to maintain deterministic behavior in code, especially in blockchain contexts.
36-39: Encouraging the use of diagrams or flowcharts for complex logic is a great practice for improving code understandability and maintainability.RELEASE_PROCESS.md (2)
49-49: Linking to the Releases document for more information on versioning is helpful for readers seeking detailed context.
80-81: The update on the support timeline for major releases 0.47 and 0.50 provides clear guidance on their end-of-life policy.
* main: feat(server/v2): introduce cometbft v2 (#20483) refactor(x/upgrade): migrate to appmodule.VersionMap (#20485) docs: code guidelines changes (#20482) feat(cosmovisor): load cosmovisor configuration from toml file (#19995) perf(math): Significantly speedup Dec quo truncate and quo Roundup (#20034) fix: Bump CometBFT versions (#20481)
* main: (120 commits) chore: update protoc-gen-swagger to protoc-gen-openapiv2 (#20448) ci: Add GitHub Action for go mod tidy and mocks (#20501) chore: Address linter issues (#20486) fix: wrap errors in auto CLI service registration (#20493) chore: fix comment (#20498) chore: fix the note box syntax error (#20499) feat(server/v2): introduce cometbft v2 (#20483) refactor(x/upgrade): migrate to appmodule.VersionMap (#20485) docs: code guidelines changes (#20482) feat(cosmovisor): load cosmovisor configuration from toml file (#19995) perf(math): Significantly speedup Dec quo truncate and quo Roundup (#20034) fix: Bump CometBFT versions (#20481) refactor(core): remove redundant ExecMode (#20322) feat(store/v2): pruning manager (#20430) chore: force reload sonar cloud (#20480) refactor(x/accounts): reuse calculated sum in `func safeAdd` (#20458) refactor: remove `defer` in loop (#20223) ci: remove livness test (#20474) build(deps): Bump bufbuild/buf-setup-action from 1.32.1 to 1.32.2 (#20477) chore: migrate a few diagrams to mermaid (#20473) ...
Description
this pr amends some documents in the repo in order to align team members and external contributors on how to work with the sdk
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit