-
Notifications
You must be signed in to change notification settings - Fork 340
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: fix typo #4180
docs: fix typo #4180
Conversation
Signed-off-by: yangquanshi <[email protected]>
📝 WalkthroughWalkthroughThe document Changes
Suggested labels
Suggested reviewers
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
🧹 Nitpick comments (1)
docs/architecture/adr-018-network-upgrades.md (1)
54-54
: Several grammar and style improvements neededPlease apply these corrections to improve readability:
- - There is a halting bug in the migration or in processing of the first transactions. This most likely would be in the form of an apphash mismatch. This becomes more problematic with delayed execution as the block (with v2 transactions) has already been committed. Immediate execution has the advantage of the apphash mismatch being realised before the data is committed. It's still however feasible to over come this but it involves nodes rolling back the previous state and re-executing the transactions using the v1 state machine (which will skip over the v2 transactions). This means node operators should be able to manually override the app version that the proposer will propose with. Lastly, if state migrations occurred between v2 and v1, a reverse migration would need to be performed which would make things especially difficult. If we are unable to fallback to the previous version and continue then the other option is to remain halted until the bug is patched and the network can update and continue + - There is a halting bug in the migration or in processing of the first transactions. This most likely would be in the form of an apphash mismatch. This becomes more problematic with delayed execution, as the block (with v2 transactions) has already been committed. Immediate execution has the advantage of the apphash mismatch being realized before the data is committed. It's still, however, feasible to overcome this, but it involves nodes rolling back the previous state and re-executing the transactions using the v1 state machine (which will skip over the v2 transactions). This means node operators should be able to manually override the app version that the proposer will propose with. Lastly, if state migrations occurred between v2 and v1, a reverse migration would need to be performed, which would make things especially difficult. If we are unable to fall back to the previous version and continue, then the other option is to remain halted until the bug is patched, and the network can update and continueChanges:
- Added missing commas in compound sentences
- Fixed spelling consistency: "realised" → "realized"
- Corrected compound word: "over come" → "overcome"
- Fixed verb form: "fallback" → "fall back"
🧰 Tools
🪛 LanguageTool
[uncategorized] ~54-~54: Possible missing comma found.
Context: ...s becomes more problematic with delayed execution as the block (with v2 transactions) has...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~54-~54: Do not mix variants of the same word (‘realise’ and ‘realize’) within a single text.
Context: ...advantage of the apphash mismatch being realised before the data is committed. It's stil...(EN_WORD_COHERENCY)
[grammar] ~54-~54: This is normally spelled as one word.
Context: ...mmitted. It's still however feasible to over come this but it involves nodes rolling back...(OVER_COMPOUNDS)
[uncategorized] ~54-~54: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...still however feasible to over come this but it involves nodes rolling back the prev...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~54-~54: Possible missing comma found.
Context: ...1, a reverse migration would need to be performed which would make things especially diff...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~54-~54: The word “fallback” is a noun. The verb is spelled with a white space.
Context: ...pecially difficult. If we are unable to fallback to the previous version and continue th...(NOUN_VERB_CONFUSION)
[uncategorized] ~54-~54: Possible missing comma found.
Context: ...to fallback to the previous version and continue then the other option is to remain halt...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~54-~54: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...o remain halted until the bug is patched and the network can update and continue -...(COMMA_COMPOUND_SENTENCE_2)
[typographical] ~54-~54: Consider adding a comma.
Context: ... and the network can update and continue - There is a bug that is detected that could ha...(IF_THERE_COMMA)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/architecture/adr-018-network-upgrades.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/architecture/adr-018-network-upgrades.md
[uncategorized] ~54-~54: Possible missing comma found.
Context: ...s becomes more problematic with delayed execution as the block (with v2 transactions) has...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~54-~54: Do not mix variants of the same word (‘realise’ and ‘realize’) within a single text.
Context: ...advantage of the apphash mismatch being realised before the data is committed. It's stil...
(EN_WORD_COHERENCY)
[grammar] ~54-~54: This is normally spelled as one word.
Context: ...mmitted. It's still however feasible to over come this but it involves nodes rolling back...
(OVER_COMPOUNDS)
[uncategorized] ~54-~54: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...still however feasible to over come this but it involves nodes rolling back the prev...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~54-~54: Possible missing comma found.
Context: ...1, a reverse migration would need to be performed which would make things especially diff...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~54-~54: The word “fallback” is a noun. The verb is spelled with a white space.
Context: ...pecially difficult. If we are unable to fallback to the previous version and continue th...
(NOUN_VERB_CONFUSION)
[uncategorized] ~54-~54: Possible missing comma found.
Context: ...to fallback to the previous version and continue then the other option is to remain halt...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~54-~54: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...o remain halted until the bug is patched and the network can update and continue -...
(COMMA_COMPOUND_SENTENCE_2)
[typographical] ~54-~54: Consider adding a comma.
Context: ... and the network can update and continue - There is a bug that is detected that could ha...
(IF_THERE_COMMA)
🔇 Additional comments (1)
docs/architecture/adr-018-network-upgrades.md (1)
54-54
: Excellent addition of failure handling scenarios!The new paragraph adds crucial information about handling halting bugs during upgrades, including apphash mismatches and the circuit breaker mechanism. This significantly improves the document's completeness regarding upgrade safety.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~54-~54: Possible missing comma found.
Context: ...s becomes more problematic with delayed execution as the block (with v2 transactions) has...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~54-~54: Do not mix variants of the same word (‘realise’ and ‘realize’) within a single text.
Context: ...advantage of the apphash mismatch being realised before the data is committed. It's stil...(EN_WORD_COHERENCY)
[grammar] ~54-~54: This is normally spelled as one word.
Context: ...mmitted. It's still however feasible to over come this but it involves nodes rolling back...(OVER_COMPOUNDS)
[uncategorized] ~54-~54: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...still however feasible to over come this but it involves nodes rolling back the prev...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~54-~54: Possible missing comma found.
Context: ...1, a reverse migration would need to be performed which would make things especially diff...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~54-~54: The word “fallback” is a noun. The verb is spelled with a white space.
Context: ...pecially difficult. If we are unable to fallback to the previous version and continue th...(NOUN_VERB_CONFUSION)
[uncategorized] ~54-~54: Possible missing comma found.
Context: ...to fallback to the previous version and continue then the other option is to remain halt...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~54-~54: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...o remain halted until the bug is patched and the network can update and continue -...(COMMA_COMPOUND_SENTENCE_2)
[typographical] ~54-~54: Consider adding a comma.
Context: ... and the network can update and continue - There is a bug that is detected that could ha...(IF_THERE_COMMA)
Overview
fix typo for docs/architecture/adr-018-network-upgrades.md