Unify version name constants in images #2096
Conversation
WalkthroughThe recent changes enhance the versioning management within the protocol's testing framework. Key modifications include introducing a dynamic versioning system, where version details are sourced from dedicated files, improving maintainability. Shell scripts and Dockerfiles have been updated to accommodate these changes, ensuring that version information is consistently referenced throughout the application. These adjustments support agile development practices by streamlining version updates and reducing hardcoded values. 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 (
|
roy-dydx
force-pushed
the
roy/versionname
branch
from
ddd92c9 to
f79d582
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (2)
protocol/testing/containertest/testnet.go (1)
12-12:CurrentVersionis not defined in theversionpackage.The
versionpackage located atprotocol/testing/version/does not defineCurrentVersion. This suggests that the package might not provide the expected versioning functionality. Please verify the intended use of this package and ensure it meets the necessary requirements.Analysis chain
Confirm the import of
versionpackage.The import of
github.com/dydxprotocol/v4-chain/protocol/testing/versionsuggests using dynamic versioning. Ensure this package provides the necessary functionality.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the presence and functionality of the `version` package. # Test: Check if `version.CurrentVersion` is defined and used correctly. ast-grep --lang go --pattern $'package version\n\nconst CurrentVersion = $_'Length of output: 540
Script:
#!/bin/bash # Description: Verify the presence and functionality of the `version` package. # Test: Check if `CurrentVersion` is defined in the `version` package. ast-grep --lang go --pattern $'const CurrentVersion = $_' -- 'protocol/testing/version/*.go'Length of output: 170
Script:
#!/bin/bash # Description: Search for the `version` package in the repository. # Find directories or files related to `version`. fd version --type d fd version.go --type fLength of output: 108
Script:
#!/bin/bash # Description: Verify the presence of `CurrentVersion` in the `version` package. # Check if `CurrentVersion` is defined in `version.go`. ast-grep --lang go --pattern $'const CurrentVersion = $_' protocol/testing/version/version.goLength of output: 94
protocol/testing/containertest/containertest.sh (1)
8-8: Update sourcing path forversion.shincontainertest.sh.The
version.shfile is located atprotocol/testing/version/version.sh. Update the sourcing path incontainertest.shto correctly reference this file to avoid runtime errors.
- Update line 8 in
protocol/testing/containertest/containertest.shtosource "../version/version.sh"Analysis chain
Ensure
version.shis correctly sourced.The addition of
source "./version.sh"suggests that version information is now dynamically sourced. Verify thatversion.shis present in the expected directory and correctly defines the necessary variables or functions.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `version.sh` is present and correctly defines the necessary variables or functions. # Test: Check for the existence of `version.sh` and inspect its contents. if [[ -f "./version.sh" ]]; then echo "version.sh exists." echo "Contents of version.sh:" cat ./version.sh else echo "version.sh is missing." fiLength of output: 78
Script:
#!/bin/bash # Search for `version.sh` in the entire repository to see if it exists elsewhere. fd version.shLength of output: 51
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (13)
- protocol/testing/containertest/Dockerfile (1 hunks)
- protocol/testing/containertest/containertest.sh (1 hunks)
- protocol/testing/containertest/testnet.go (3 hunks)
- protocol/testing/containertest/testnet_test.go (2 hunks)
- protocol/testing/mainnet/Dockerfile (1 hunks)
- protocol/testing/mainnet/vars.sh (1 hunks)
- protocol/testing/testnet/Dockerfile (1 hunks)
- protocol/testing/testnet/vars.sh (1 hunks)
- protocol/testing/version/README.md (1 hunks)
- protocol/testing/version/VERSION_CURRENT (1 hunks)
- protocol/testing/version/VERSION_PREUPGRADE (1 hunks)
- protocol/testing/version/version.go (1 hunks)
- protocol/testing/version/version.sh (1 hunks)
Files skipped from review due to trivial changes (5)
- protocol/testing/containertest/Dockerfile
- protocol/testing/version/README.md
- protocol/testing/version/VERSION_CURRENT
- protocol/testing/version/VERSION_PREUPGRADE
- protocol/testing/version/version.go
Additional context used
Shellcheck
protocol/testing/version/version.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 1-1: CURRENT_VERSION appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 6-6: version_to_url appears unused. Verify use (or export if used externally).
(SC2034)
Additional comments not posted (8)
protocol/testing/mainnet/vars.sh (1)
4-4: Verify the impact of sourcingversion.sh.The change from sourcing
current_version.shtoversion.shsuggests a centralized approach to versioning. Ensure thatversion.shprovides all the necessary version information previously available incurrent_version.shand that no dependent scripts are broken by this change.protocol/testing/mainnet/Dockerfile (1)
6-6: Verify the inclusion of multiple files from theversiondirectory.The change from copying a single script to copying the entire
versiondirectory suggests a shift to using multiple version-related resources. Ensure that the Docker image correctly utilizes these resources and that no unnecessary files are included.protocol/testing/testnet/Dockerfile (1)
6-6: Verify the inclusion of multiple files from theversiondirectory.This change mirrors the mainnet Dockerfile. Ensure the Docker image correctly uses the version-related resources and that no unnecessary files are included.
protocol/testing/version/version.sh (2)
1-1: Verify the usage ofCURRENT_VERSION.The variable
CURRENT_VERSIONappears unused in this file. Ensure it is used or exported if needed elsewhere.Verification successful
CURRENT_VERSIONis used in the codebase.The variable
CURRENT_VERSIONis used in constructing a path forcurrent_version_path. Ensure that its usage aligns with the intended functionality.
- Usage found in constructing paths:
current_version_path="$HOME/cosmovisor/upgrades/$CURRENT_VERSION/bin"Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if `CURRENT_VERSION` is used elsewhere in the codebase. # Test: Search for the usage of `CURRENT_VERSION`. Expect: Occurrences of the variable usage. rg --type shell 'CURRENT_VERSION'Length of output: 63
Script:
#!/bin/bash # Description: Find all shell script files and search for the usage of `CURRENT_VERSION`. # Find all shell script files by looking for files with a .sh extension fd --extension sh --exec rg 'CURRENT_VERSION' {}Length of output: 234
Tools
Shellcheck
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 1-1: CURRENT_VERSION appears unused. Verify use (or export if used externally).
(SC2034)
6-6: Verify the usage ofversion_to_url.The associative array
version_to_urlappears unused in this file. Ensure it is used or exported if needed elsewhere.Verification successful
version_to_urlis used within the script.The associative array
version_to_urlis indeed used in the script. It is iterated over to retrieve URLs based on versions.
- The array is declared and populated.
- It is used in a loop to access URLs associated with versions.
No further action is needed regarding its usage.
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 of the array usage. rg --type shell 'version_to_url'Length of output: 62
Script:
#!/bin/bash # Description: Correctly search for the usage of `version_to_url` in the codebase. # Find all shell script files and search for `version_to_url` within them. fd -e sh -x rg 'version_to_url' {}Length of output: 419
Tools
Shellcheck
[warning] 6-6: version_to_url appears unused. Verify use (or export if used externally).
(SC2034)
protocol/testing/testnet/vars.sh (1)
4-4: Ensure compatibility with the new version script.The script now sources
version.sh. Verify that all necessary variables and functions are correctly imported and utilized.protocol/testing/containertest/testnet.go (1)
159-159: Useversion.CurrentVersiondynamically.The use of
version.CurrentVersionreplaces a hardcoded version, improving maintainability. Ensure that this dynamic approach is consistent throughout the codebase.protocol/testing/containertest/testnet_test.go (1)
286-286: Verifyversion.CurrentVersionusage.The change from
UpgradeToVersiontoversion.CurrentVersionindicates a move to a centralized versioning system. Ensure thatversion.CurrentVersionis correctly defined and used consistently throughout the codebase.Verification successful
version.CurrentVersionis correctly defined and used.The
CurrentVersionvariable is defined as astringin theversionpackage and is used consistently across the codebase in contexts related to versioning.
- Defined in
protocol/testing/version/version.go.- Used in
protocol/testing/containertest/testnet_test.goandprotocol/testing/containertest/testnet.go.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `version.CurrentVersion` is correctly defined and used. # Test: Search for the definition of `CurrentVersion` in the `version` package. ast-grep --lang go --pattern $'package version\n\nvar CurrentVersion = $_'Length of output: 536
Script:
#!/bin/bash # Corrected script to find the definition of `CurrentVersion` in the `version` package. # Search for the definition of `CurrentVersion` in Go files. ast-grep --lang go --pattern 'var CurrentVersion = $_' # Search for the usage of `CurrentVersion` across the codebase. rg 'CurrentVersion' --type go --context 2Length of output: 1057
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- protocol/testing/version/version.sh (1 hunks)
Additional context used
Shellcheck
protocol/testing/version/version.sh
[warning] 2-2: CURRENT_VERSION appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 7-7: version_to_url appears unused. Verify use (or export if used externally).
(SC2034)
Additional comments not posted (7)
protocol/testing/version/version.sh (7)
1-1: Shebang is correctly specified.The shebang line
#!/bin/bashis correctly added to specify the shell interpreter.
3-3: Assignment ofPREUPGRADE_VERSIONis correct.The variable
PREUPGRADE_VERSIONis correctly assigned and used in the URL mapping.
4-4: Blank line for readability is appropriate.The blank line improves readability by separating variable declarations from the following section.
5-5: Comment provides useful context.The comment clearly explains the purpose of the version-to-URL mapping.
2-2: Verify usage ofCURRENT_VERSION.The variable
CURRENT_VERSIONis assigned but not used in the script. Verify if it should be used or exported for external use.Verification successful
CURRENT_VERSIONis used in other scripts.The variable
CURRENT_VERSIONis indeed used in other scripts within the codebase, specifically inprotocol/testing/testnet/testnet.shandprotocol/testing/mainnet/mainnet.sh. This indicates that the assignment inprotocol/testing/version/version.shis necessary for these scripts to function correctly. No changes are needed regarding its usage.
protocol/testing/testnet/testnet.sh: UsesCURRENT_VERSIONto setcurrent_version_path.protocol/testing/mainnet/mainnet.sh: UsesCURRENT_VERSIONto setcurrent_version_path.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify usage of `CURRENT_VERSION` in the codebase. # Test: Search for the usage of `CURRENT_VERSION`. Expect: Occurrences where it is used or exported. rg --type shell 'CURRENT_VERSION'Length of output: 63
Script:
#!/bin/bash # Description: Verify usage of `CURRENT_VERSION` in the codebase. # Test: Search for the usage of `CURRENT_VERSION`. Expect: Occurrences where it is used or exported. rg --type sh 'CURRENT_VERSION'Length of output: 322
Tools
Shellcheck
[warning] 2-2: CURRENT_VERSION appears unused. Verify use (or export if used externally).
(SC2034)
6-6: Verify usage ofversion_to_url.The associative array
version_to_urlis declared but not used in the script. Verify if it should be used or exported for external use.Verification successful
version_to_urlis used in multiple scripts.The associative array
version_to_urlis indeed used in other parts of the codebase:
- It is populated in
protocol/testing/version/version.sh.- It is accessed in loops in
protocol/testing/testnet/testnet.shandprotocol/testing/mainnet/mainnet.sh.This indicates that the array is utilized properly across different scripts.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify usage of `version_to_url` in the codebase. # Test: Search for the usage of `version_to_url`. Expect: Occurrences where it is used or exported. rg --type shell 'version_to_url'Length of output: 62
Script:
#!/bin/bash # Description: Verify usage of `version_to_url` in the codebase. # Test: Search for the usage of `version_to_url`. Expect: Occurrences where it is used or exported. rg 'version_to_url'Length of output: 618
7-7: Initialization ofversion_to_urlis correct, but verify its usage.The initialization logic for
version_to_urlis correct, usingPREUPGRADE_VERSION. However, ensure that this array is used or exported.Verification successful
version_to_urlis correctly initialized and used in the codebase.The
version_to_urlarray is initialized inversion.shand used in bothtestnet.shandmainnet.sh. This confirms that it is utilized as intended.
- Files and usage:
protocol/testing/version/version.sh: Initialization.protocol/testing/testnet/testnet.sh: Usage in a loop.protocol/testing/mainnet/mainnet.sh: Usage in a loop.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify initialization and usage of `version_to_url`. # Test: Search for the usage of `version_to_url`. Expect: Occurrences where it is used or exported. rg --type shell 'version_to_url'Length of output: 62
Script:
#!/bin/bash # Description: Verify initialization and usage of `version_to_url`. # Test: Search for the usage of `version_to_url` in all files. rg 'version_to_url'Length of output: 618
Tools
Shellcheck
[warning] 7-7: version_to_url appears unused. Verify use (or export if used externally).
(SC2034)
|
!Mergifyio backport release/protocol/v6.x |
|
@Mergifyio backport release/protocol/v6.x |
✅ Backports have been created
|
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> (cherry picked from commit 3540618)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> (cherry picked from commit 3540618)
Co-authored-by: roy-dydx <[email protected]>
Changelist
Version names for building images moved to a single source of truth.
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
New Features
Bug Fixes
Chores
Documentation