allow configuring full preupgrade version name #2108
Conversation
WalkthroughThe recent changes enhance version control for the download process of pre-upgrade binaries. A new variable, Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configuration 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- protocol/testing/containertest/containertest.sh (1 hunks)
- protocol/testing/version/README.md (1 hunks)
- protocol/testing/version/VERSION_FULL_NAME_PREUPGRADE (1 hunks)
- protocol/testing/version/version.sh (1 hunks)
Files skipped from review due to trivial changes (1)
- protocol/testing/version/VERSION_FULL_NAME_PREUPGRADE
Additional context used
Shellcheck
protocol/testing/version/version.sh
[warning] 8-8: version_to_url appears unused. Verify use (or export if used externally).
(SC2034)
Additional comments not posted (4)
protocol/testing/version/version.sh (2)
4-4: Introduction ofPREUPGRADE_VERSION_FULL_NAME.The new variable
PREUPGRADE_VERSION_FULL_NAMEis introduced to enhance clarity in version naming. Ensure this variable is correctly populated in theVERSION_FULL_NAME_PREUPGRADEfile.
8-8: Verify the usage ofversion_to_url.The static analysis tool indicates that
version_to_urlappears unused. Verify if this is intentional or if the mapping is used elsewhere.Verification successful
version_to_urlis used in the codebase.The
version_to_urlvariable is utilized in the following scripts, confirming its usage:
protocol/testing/testnet/testnet.sh: Iterates overversion_to_urlkeys.protocol/testing/mainnet/mainnet.sh: Iterates overversion_to_urlkeys.The initial concern about it being unused is incorrect.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if `version_to_url` is used elsewhere in the codebase. # Test: Search for the usage of `version_to_url`. Expect: Occurrences in other scripts or functions. rg --type shell 'version_to_url'Length of output: 62
Script:
#!/bin/bash # Description: Verify if `version_to_url` is used elsewhere in the codebase. # Test: Search for the usage of `version_to_url`. Expect: Occurrences in other scripts or functions. rg --type sh 'version_to_url'Length of output: 648
Tools
Shellcheck
[warning] 8-8: version_to_url appears unused. Verify use (or export if used externally).
(SC2034)
protocol/testing/version/README.md (1)
1-5: Enhanced documentation for version naming conventions.The introduction of
VERSION_FULL_NAME_PREUPGRADEin the documentation provides clarity on versioning schemes used for upgrade tests and internal nodes. This improves usability for developers.protocol/testing/containertest/containertest.sh (1)
172-172: Updated URL construction indownload_preupgrade_binary.The change to use
PREUPGRADE_VERSION_FULL_NAMEin the URL construction aligns with the new naming convention, improving clarity in version identification.
| @@ -1,7 +1,8 @@ | |||
| #!/bin/bash | |||
| CURRENT_VERSION=$(<VERSION_CURRENT) | |||
| PREUPGRADE_VERSION=$(<VERSION_PREUPGRADE) | |||
| PREUPGRADE_VERSION_FULL_NAME=$(<VERSION_FULL_NAME_PREUPGRADE) | |||
|
|
|||
There was a problem hiding this comment.
Do you think instead of having three files VERSION_CURRENT, VERSION_PREUPGRADE, VERSION_PREUPGRADE - it would be better to have a single file and source that file here?
There was a problem hiding this comment.
Don't think that would work too well with the embed directive I have in version.go in this directory.
|
@Mergifyio backport release/protocol/v6.x |
✅ Backports have been created
|
(cherry picked from commit 5998dfc) # Conflicts: # protocol/testing/version/README.md # protocol/testing/version/version.sh
(cherry picked from commit 5998dfc)
(cherry picked from commit 5998dfc)
Co-authored-by: roy-dydx <[email protected]>
Changelist
[Describe or list the changes made in this PR]
Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
VERSION_FULL_NAME_PREUPGRADE).VERSION_FULL_NAME_PREUPGRADE.