-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
docs: ADR 073: update to accepted and add to README.md #20619
Conversation
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe update introduces a new Architectural Decision Record (ADR 073) titled "Built-in In-process Indexer" in the Changes
Sequence Diagram(s)No sequence diagrams are necessary for these changes as they primarily involve 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 (
|
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 (3)
docs/architecture/README.md (1)
Line range hint
28-28
: Consider adding an article before "streamlined summary" to improve grammatical correctness.- The spec is much more compressed and streamlined summary of everything as it stands today. + The spec is a much more compressed and streamlined summary of everything as it stands today.docs/architecture/adr-073-indexer.md (2)
Line range hint
50-50
: Consider rephrasing to avoid redundancy. Replace "point in time" with "point".- ... complete query index starting from any point in time without necessarily needing to replay a... + ... complete query index starting from any point without necessarily needing to replay a...
Line range hint
71-71
: Correct the verb form for better grammatical accuracy.- ...ecode it into logical packets which can consumed by an indexer. + ...ecode it into logical packets which can be consumed by an indexer.
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (2)
- docs/architecture/README.md (1 hunks)
- docs/architecture/adr-073-indexer.md (1 hunks)
Additional context used
Path-based instructions (2)
docs/architecture/README.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/architecture/adr-073-indexer.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
LanguageTool
docs/architecture/README.md
[uncategorized] ~28-~28: Possible missing article found. (AI_HYDRA_LEO_MISSING_A)
Context: ...hitecture of something new. The spec is much more compressed and streamlined summary...docs/architecture/adr-073-indexer.md
[style] ~50-~50: This phrase is redundant. Consider writing “point” or “time”. (MOMENT_IN_TIME)
Context: ... complete query index starting from any point in time without necessarily needing to replay a...
[grammar] ~71-~71: The past tense is already expressed by ‘can’. Did you mean “consume”? (HOW_DID_THAT_HAPPEN)
Context: ...ecode it into logical packets which can consumed by an indexer. It should define an inte...
[style] ~77-~77: Consider a shorter alternative to avoid wordiness. (IN_ORDER_TO_PREMIUM)
Context: ...changes tocollections
will be needed in order to expose for decoding functionality and s...
[style] ~80-~80: Consider a shorter alternative to avoid wordiness. (IN_ORDER_TO_PREMIUM)
Context: ...onfigured at the module or node level. In order to support indexing from any height (N3
)...
[grammar] ~91-~91: The plural noun “batteries” cannot be used with the article “a”. Did you mean “a full batterie”, “a full battery” or “full batteries”? (A_NNS)
Context: ...thout needing to go through Comet. For a full batteries included, client friendly query experie...
[style] ~93-~93: Consider a shorter alternative to avoid wordiness. (IN_ORDER_TO_PREMIUM)
Context: ... the query indexer in the configuration in order to provide a full-featured query experienc...
[misspelling] ~101-~101: Use ‘short-term’ only as an adjective. For the adverbial phrase, use “short term”. (THE_SHORT_TERM)
Context: ... this would simplify our efforts in the short-term, it still doesn't provide a full-featur...
[grammar] ~103-~103: Possible agreement error. The noun refactoring seems to be countable; consider using: “a lot of module refactorings”. (A_LOT_OF_NN)
Context: ...e-based indexer, however, would require a lot of module refactoring, likely custom code, and wouldn't take ...
[style] ~104-~104: For conciseness, consider replacing this expression with an adverb. (AT_THE_MOMENT)
Context: ...ecodable schema forcollections
which at the moment is fairly complex. It is easier to use ...
[uncategorized] ~104-~104: A comma may be missing after the conjunctive/linking adverb ‘Also’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Context: ...uage for a separate process to consume. Also we want to provide a more batteries-inc...
[style] ~105-~105: To elevate your writing, try using a synonym here. (HARD_TO)
Context: ...ient only indexes in state, it would be hard to configure custom indexes and views, ...
[uncategorized] ~115-~115: The adjective “client-friendly” is spelled with a hyphen. (NN_FRIENDLY_HYPHEN)
Context: ...g ofcollections
andorm
schemas is client friendly. Also, because we are separating the d...
[uncategorized] ~129-~129: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...ndU4.3
likely require a full archive node which is out of scope of this design. O...
[formatting] ~129-~129: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause. (COMMA_BEFORE_BECAUSE)
Context: ...concerned out of the scope of the design, because it is either much more complex from an ...
Markdownlint
docs/architecture/adr-073-indexer.md
41-41: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
42-42: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
43-43: Expected: 2; Actual: 4 (MD007, ul-indent)
Unordered list indentation
40-40: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
111-111: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
95-95: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
Additional comments not posted (2)
docs/architecture/README.md (1)
65-65
: The addition of ADR 073 to the "Accepted" section is correctly implemented.docs/architecture/adr-073-indexer.md (1)
9-9
: The update of ADR 073's status to "Accepted Not Implemented" is correctly implemented.
* main: refactor(x/auth): Fix system test (#20531) feat(crypto): add blst (#20296) docs: Update high level overview and introduction (#20535) refactor(x/core): remove test (#20624) feat(x/mint)!: Replace InflationCalculationFn with MintFn + simple epoch minting (#20363) docs: remove duplicate words (#20622) feat: prep for errors v2 (#20539) chore: reduce default inflation (#20606) refactor(store): add miss defer (#20602) chore: use comet api pkg instead of comet alias (#20614) chore: write gentx info to cmd.ErrOrStderr (#20616) docs: ADR 073: update to accepted and add to README.md (#20619) chore(proto): change future extracted modules version from v1.0.0 to v0.2.0 (#20600) fix: remove some duplicate words (#20605) feat(stf): port simappv2 changes (#20587) chore: bring patch changelogs to main (#20599)
Description
#20532 got merged without getting added to README.md. This PR fixes that and also moves the ADR status to Accepted. Is that okay @tac0turtle ?
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.md
Reviewers 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