feat: add cli for canceling a software upgrade #5
Conversation
WalkthroughThe pull request introduces a new CLI command, Changes
Possibly related PRs
Poem
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
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
![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
🧹 Nitpick comments (3)
CHANGELOG.md (1)
3-11: Convert emphasis to proper heading.The release date should use a proper heading format instead of emphasis to maintain consistency with markdown best practices.
-*Feb 10, 2024* +### Feb 10, 2024🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
5-5: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
.changelog/v1.0.2/summary.md (1)
1-3: Convert emphasis to proper heading.The release date should use a proper heading format instead of emphasis to maintain consistency with markdown best practices.
-*Feb 10, 2024* +### Feb 10, 2024🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
1-1: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
.changelog/v1.0.1/improvements/4-add-helper-commands.md (1)
1-2: Suggestion: Correct verb tense and improve article usage for consistency.The current changelog line uses "recover" which does not parallel the gerund "broadcasting." Consider revising the sentence for clarity and consistency, for example:
- Add helper commands for broadcasting software upgrade and recover client messages. ([#4](https://github.com/noble-assets/authority/pull/4)) + Add helper commands for broadcasting a software upgrade and recovering client messages. ([#4](https://github.com/noble-assets/authority/pull/4))This change improves clarity and maintains a consistent structure across the listed actions.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~1-~1: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...s for broadcasting software upgrade and recover client messages. ([#4](https://github.c...(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changelog/v1.0.1/improvements/4-add-helper-commands.md(1 hunks).changelog/v1.0.2/improvements/5-add-helper-command.md(1 hunks).changelog/v1.0.2/summary.md(1 hunks)CHANGELOG.md(2 hunks)client/cli/tx.go(2 hunks)e2e/upgrade_test.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- client/cli/tx.go
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
5-5: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
.changelog/v1.0.2/summary.md
1-1: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
🪛 LanguageTool
CHANGELOG.md
[uncategorized] ~21-~21: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...s for broadcasting software upgrade and recover client messages. ([#4](https://github.c...
(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
.changelog/v1.0.1/improvements/4-add-helper-commands.md
[uncategorized] ~1-~1: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...s for broadcasting software upgrade and recover client messages. ([#4](https://github.c...
(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: test (TestScheduleAndCancelUpgrade)
- GitHub Check: test (TestParameterUpdate)
- GitHub Check: test (TestOwnershipTransfer)
- GitHub Check: test (TestBeginBlocker)
- GitHub Check: test (TestClientSubstitution)
🔇 Additional comments (6)
e2e/upgrade_test.go (4)
32-33: LGTM!The function name and comment accurately reflect the expanded functionality of testing both scheduling and cancelling upgrades.
64-80: LGTM!The test cases for cancelling software upgrade are well-structured and consistent with the existing patterns:
- Test unauthorized access with proper error validation
- Test authorized access with success validation
82-82: LGTM!The final state verification correctly ensures that the upgrade was cancelled by checking for empty version and zero height.
44-44: Verify the impact of removing the--chain-idparameter.The command has been simplified by removing the
--chain-idparameter. Please ensure this change doesn't affect the command's functionality in different chain environments.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if the command works without chain-id in different environments. # Search for other occurrences of the command to ensure consistency rg -A 5 'authority software-upgrade'Length of output: 38
Let's run a follow-up script to gather more context about the use (or non-use) of the
--chain-idparameter in the repository. In particular, we want to inspect the relevant section of the test file and search for any occurrences of the--chain-idflag.
🏁 Script executed:
#!/bin/bash echo "Searching for '--chain-id' flag in the entire repository:" rg "--chain-id" || echo "No '--chain-id' occurrences found." echo "" echo "Verifying the command definition in e2e/upgrade_test.go (lines 40-50):" awk 'NR>=40 && NR<=50 {print NR, $0}' e2e/upgrade_test.goLength of output: 1264
The removal of the
--chain-idparameter does not appear to break functionality.
- A repository-wide search for the
--chain-idflag (now using proper quoting) revealed no occurrences, confirming that the parameter was not used elsewhere.- The test in
e2e/upgrade_test.goon line 44 only relies on the explicitly provided parameters, and there’s no evidence of chain-specific code expecting--chain-id..changelog/v1.0.2/improvements/5-add-helper-command.md (1)
1-1: LGTM!The changelog entry is well-formatted and clearly describes the new functionality with the correct PR reference.
CHANGELOG.md (1)
21-21: LGTM!The improvement entry is well-formatted and consistent with the changelog style.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~21-~21: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...s for broadcasting software upgrade and recover client messages. ([#4](https://github.c...(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.changelog/v1.0.2/summary.md (1)
1-3: Markdown Formatting SuggestionThe release date "Feb 10, 2025" is formatted using emphasis which may trigger markdownlint MD036. If you intend for this to be a heading or a prominently formatted date, consider using a proper heading (for example,
# Feb 10, 2025) or an alternative styling.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
1-1: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
CHANGELOG.md (1)
21-21: Language Suggestion for Verb FormThe entry on line 21 reads:
- Add helper commands for broadcasting software upgrade and recover client messages.
Consider rephrasing it for clarity. For example, "Add helper commands to broadcast software upgrade and recover client messages" may read more naturally.🧰 Tools
🪛 LanguageTool
[uncategorized] ~21-~21: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...s for broadcasting software upgrade and recover client messages. ([#4](https://github.c...(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changelog/v1.0.2/summary.md(1 hunks)CHANGELOG.md(2 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
5-5: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
.changelog/v1.0.2/summary.md
1-1: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
🪛 LanguageTool
CHANGELOG.md
[uncategorized] ~21-~21: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...s for broadcasting software upgrade and recover client messages. ([#4](https://github.c...
(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: prepare
- GitHub Check: build
🔇 Additional comments (2)
CHANGELOG.md (2)
3-8: New Version Entry for v1.0.2The new changelog entry for version v1.0.2 is clearly documented with the appropriate release date and description. Note that the release date on line 5 is formatted with emphasis; if a heading style is preferred, you might consider adjusting it accordingly.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
5-5: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
11-12: Helper Command Improvement EntryThe improvement entry for adding a helper command to broadcast a cancel software upgrade message is well stated and appropriately references pull request #5.
johnletey
changed the title
Summary by CodeRabbit