gha: Enable docker production build in PR #6715
Conversation
📝 WalkthroughWalkthroughThe pull request modifies GitHub Actions workflows for building Docker images and DAppNode packages. In Changes
Possibly related PRs
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
.github/workflows/build-docker.yaml (2)
Line range hint
134-134: Fix syntax error in deployment trigger conditionThe current condition has a syntax error and might not evaluate as intended.
Apply this fix:
- - name: Trigger deploy workflow if needed - if: (contains(github.event.pull_request.labels.*.name, 'deploy_nodes') || inputs.production ) && contains(inputs.package,'hoprd' && vars.CONTINUOUS_DEPLOYMENT_ENABLED == 'true') + - name: Trigger deploy workflow if needed + if: (contains(github.event.pull_request.labels.*.name, 'deploy_nodes') || inputs.production) && contains(inputs.package, 'hoprd') && vars.CONTINUOUS_DEPLOYMENT_ENABLED == 'true'This fix:
- Properly separates the contains() check for 'hoprd'
- Correctly applies the logical operators
- Makes the condition more readable
🧰 Tools
🪛 actionlint (1.7.4)
67-67: shellcheck reported issue in this script: SC2078:error:2:7: This expression is constant. Did you forget a $ somewhere?
(shellcheck)
67-67: shellcheck reported issue in this script: SC2078:error:2:35: This expression is constant. Did you forget a $ somewhere?
(shellcheck)
67-67: shellcheck reported issue in this script: SC2129:style:9:3: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:9:38: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:10:44: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:11:95: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:14:29: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2001:style:15:16: See if you can use ${variable//search/replace} instead
(shellcheck)
67-67: shellcheck reported issue in this script: SC2001:style:16:19: See if you can use ${variable//search/replace} instead
(shellcheck)
67-67: shellcheck reported issue in this script: SC2129:style:17:3: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:17:38: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:18:44: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:19:95: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:28:42: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:30:65: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:32:66: Double quote to prevent globbing and word splitting
(shellcheck)
Line range hint
1-140: Consider workflow improvementsWhile the workflow functions correctly, consider these improvements for maintainability:
- Add comments explaining the purpose and format of different image tags
- Add a step to verify required environment variables at the start of the workflow
Example addition for environment variable verification:
- name: Verify required environment variables run: | required_vars=( "DOCKER_IMAGE_REGISTRY" "BRANCH_MASTER_RELEASE_NAME" "BRANCH_RELEASE_RELEASE_NAME" "CONTINUOUS_DEPLOYMENT_ENABLED" ) for var in "${required_vars[@]}"; do if [[ -z "${!var}" ]]; then echo "::error::Required environment variable $var is not set" exit 1 fi done🧰 Tools
🪛 actionlint (1.7.4)
67-67: shellcheck reported issue in this script: SC2078:error:2:7: This expression is constant. Did you forget a $ somewhere?
(shellcheck)
67-67: shellcheck reported issue in this script: SC2078:error:2:35: This expression is constant. Did you forget a $ somewhere?
(shellcheck)
67-67: shellcheck reported issue in this script: SC2129:style:9:3: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:9:38: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:10:44: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:11:95: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:14:29: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2001:style:15:16: See if you can use ${variable//search/replace} instead
(shellcheck)
67-67: shellcheck reported issue in this script: SC2001:style:16:19: See if you can use ${variable//search/replace} instead
(shellcheck)
67-67: shellcheck reported issue in this script: SC2129:style:17:3: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:17:38: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:18:44: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:19:95: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:28:42: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:30:65: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:32:66: Double quote to prevent globbing and word splitting
(shellcheck)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/build-docker.yaml(1 hunks).github/workflows/build.yaml(1 hunks)
🔇 Additional comments (2)
.github/workflows/build.yaml (1)
42-42: LGTM: Improved YAML formatting
The change improves readability by explicitly using YAML list format for the job dependency.
.github/workflows/build-docker.yaml (1)
69-71: LGTM: Enhanced production build control
The condition correctly implements the PR objective by enabling production builds via either the input parameter or the 'docker-production-build' label.
By default PRs build docker images which include a hoprd debug build. This is done to have faster build times in the CI. This PR adds the option to build production images by adding the Github label
docker-production-buildto a PR and rebuilding the image.