-
Notifications
You must be signed in to change notification settings - Fork 368
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
chore(deps): upgrade to celestia-core v1.37.0-tm-v0.34.29 #3607
Conversation
WalkthroughWalkthroughThe updates primarily focus on incrementing the version of Go from Changes
Recent review detailsConfiguration used: .coderabbit.yaml Files ignored due to path filters (3)
Files selected for processing (10)
Files skipped from review due to trivial changes (9)
Additional context usedHadolint
Additional comments not posted (1)
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 Configration File (
|
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: 1
Outside diff range and nitpick comments (2)
test/testground/Dockerfile (2)
Line range hint
50-50
: Optimize Docker layer caching and pin package versions.Use the
--no-cache
switch when installing packages withapk
to avoid the need to use--update
and remove/var/cache/apk/*
when done. Also, pin versions of packages to ensure reproducibility.- RUN apk add gcompat + RUN apk add --no-cache gcompat=1.0.0 # Replace 1.0.0 with the exact version you need
[!TIP]
Codebase VerificationCgoEnabled variable is not defined elsewhere in the Dockerfile.
The search results indicate that
CgoEnabled
is only used and not defined elsewhere in the Dockerfile or related files. Therefore, assigning a default value as suggested in the review comment is necessary to avoid potential issues during the build process.
- File:
test/testground/Dockerfile
- Lines: 65
Suggested Change:
- && CGO_ENABLED=${CgoEnabled} GOOS=linux GOARCH=amd64 go build -o ${PLAN_DIR}/testplan.bin ${BUILD_TAGS} ${TESTPLAN_EXEC_PKG} + && CGO_ENABLED=${CgoEnabled:-0} GOOS=linux GOARCH=amd64 go build -o testplan.bin ${BUILD_TAGS} ${TESTPLAN_EXEC_PKG}Analysis chain
Line range hint
53-53
: Use WORKDIR instead of repetitivecd
commands.Using
WORKDIR
simplifies the Dockerfile and avoids potential errors with directory paths. Also, ensure thatCgoEnabled
is correctly assigned before use.- RUN cd ${PLAN_DIR} \ + WORKDIR ${PLAN_DIR} - && CGO_ENABLED=${CgoEnabled} GOOS=linux GOARCH=amd64 go build -o ${PLAN_DIR}/testplan.bin ${BUILD_TAGS} ${TESTPLAN_EXEC_PKG} + && CGO_ENABLED=${CgoEnabled:-0} GOOS=linux GOARCH=amd64 go build -o testplan.bin ${BUILD_TAGS} ${TESTPLAN_EXEC_PKG}Also applies to: 63-63, 68-68
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify if CgoEnabled is defined elsewhere in the Dockerfile or Docker context. grep -rnw '/path/to/Docker/context' -e 'CgoEnabled'Length of output: 106
Script:
#!/bin/bash # Search for the definition of CgoEnabled within the Dockerfile and relevant files in the repository. # Adjusting the search to the actual repository context. grep -rnw '.' -e 'CgoEnabled'Length of output: 186
@@ -1,6 +1,6 @@ | |||
# BUILD_BASE_IMAGE is the base image to use for the build. It contains a rolling | |||
# accumulation of Go build/package caches. | |||
ARG BUILD_BASE_IMAGE=docker.io/golang:1.22.3-alpine3.19 | |||
ARG BUILD_BASE_IMAGE=docker.io/golang:1.22.4-alpine3.19 |
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.
Specify the exact version of the alpine image.
Always tag the version of an image explicitly to ensure reproducibility and avoid surprises from unintended upgrades. The current tag alpine3.19
does not specify the patch version.
- ARG BUILD_BASE_IMAGE=docker.io/golang:1.22.4-alpine3.19
+ ARG BUILD_BASE_IMAGE=docker.io/golang:1.22.4-alpine3.19.0
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
ARG BUILD_BASE_IMAGE=docker.io/golang:1.22.4-alpine3.19 | |
ARG BUILD_BASE_IMAGE=docker.io/golang:1.22.4-alpine3.19.0 |
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.
LGTM!
Motivation: because celestia-core v1.37.0 uses NMT v0.21.0 which contains this fix: celestiaorg/nmt#253