Update indexer field value - #1975
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request updates the consensus node guide by modifying the configuration setting in the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🪧 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
CodeRabbit Configuration File (
|
adlerjohn
added
bug
|
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
how-to-guides/consensus-node.md (2)
164-172: Update transaction indexer configuration explanationThe updated code now sets the
indexerto"null", which is explained as a lightweight option that avoids redundant storage compared to"kv". Please double-check the accompanying explanation to ensure it clearly communicates that opting for"null"will disable transaction indexing—which is ideal when you do not require transaction query functionality—and that the trade-off is the loss of such queries. If users might need to run transaction queries, consider adding a brief note about potential implications.
518-518: Minor FAQ formatting updateThe FAQ header now reads:
### `+2/3 committed an invalid block: wrong Block.Header.Version`This formatting adjustment improves consistency. Ensure that all FAQ headers follow a similar style for a uniform user experience.
jcstein
left a comment
jcstein
left a comment
There was a problem hiding this comment.
LGTM, should we wait to smorge until celestiaorg/celestia-core#1720 is done?
|
No, no need to wait, they're orthogonal. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
how-to-guides/consensus-node.md (1)
471-481: List Numbering & Punctuation Review in Indexer Options:
The available indexer options list uses a discontinuous numbering sequence (1, then 3, then 4). If the omission (i.e., no option "2") is intentional (for example, to indicate a deprecated option or reserved slot), a brief note might help avoid confusion. Additionally, minor static analysis hints flagged loose punctuation on lines ~474 and ~478; although these are subjective, consider a quick review to ensure the formatting aligns with your documentation style guidelines.🧰 Tools
🪛 LanguageTool
[uncategorized] ~474-~474: Loose punctuation mark.
Context: ...hoose this option to save space. 3.kv: This is the simplest indexer, backed by...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~478-~478: Loose punctuation mark.
Context: ...basic queries on transactions. 4.psql: This indexer is backed by PostgreSQL. W...(UNLIKELY_OPENING_PUNCTUATION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
how-to-guides/consensus-node.md(3 hunks)
🧰 Additional context used
🪛 LanguageTool
how-to-guides/consensus-node.md
[uncategorized] ~474-~474: Loose punctuation mark.
Context: ...hoose this option to save space. 3. kv: This is the simplest indexer, backed by...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~478-~478: Loose punctuation mark.
Context: ...basic queries on transactions. 4. psql: This indexer is backed by PostgreSQL. W...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (1)
how-to-guides/consensus-node.md (1)
164-173: Updated Indexer Configuration Instruction:
The instructions now correctly advise settingindexer = "null"in theconfig.tomlfile, aligning with the PR’s objectives to reduce redundant blob storage from thekvindexer. The explanation clearly states that while full indexing isn’t disabled, only transaction status indexing is enabled. Please confirm that users understand this nuance.
3 participants
The
nullindexer is sufficient since it actually does index transaction hashes to transaction statues (which is sufficient for a bridge node).kvindexes additional data that isn't needed by bridge nodes.Related:
Summary by CodeRabbit
Summary by CodeRabbit