Repository navigation
Dockerfiles: set OBA_VERSION after the package installs - #151
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 23 minutes. View limit details
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Builder-stage declarations bundler/Dockerfile, oba/Dockerfile |
The bundler builder declares OBA_VERSION after copying project files. The oba builder declares it before Maven installs dependencies. |
Runtime-stage declarations bundler/Dockerfile, oba/Dockerfile |
The bundler and server stages declare OBA_VERSION after user setup. |
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Low
Suggested reviewers: aaronbrethorst
Merge Risk: 
0f664
-
The relocation preserves build-time and runtime OBA_VERSION behavior while avoiding unnecessary package-install invalidation.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title accurately describes moving OBA_VERSION declarations later in the Dockerfiles, especially after package installation in the runtime stages. It does not describe the builder-stage placement p… Linked Issues check Direct issue [ #145] requires movingARG OBA_VERSIONandENV OBA_VERSIONin four stages. The diff moves the pair after dependency-file copies and before Maven use inbundler/Dockerfilebuilder, a…Out of Scope Changes check The whole-PR diff changes only the placement of ARG OBA_VERSIONandENV OBA_VERSIONin the four stages named by [#145]. It contains no unrelated source, dependency, configuration, or behavior chan…Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Full details: Title check
-
Explanation
The title accurately describes moving OBA_VERSION declarations later in the Dockerfiles, especially after package installation in the runtime stages. It does not describe the builder-stage placement precisely, but it remains clearly related to the main change.
Full details: Linked Issues check
-
Explanation
Direct issue [
#145] requires movingARG OBA_VERSIONandENV OBA_VERSIONin four stages. The diff moves the pair after dependency-file copies and before Maven use inbundler/Dockerfilebuilder, and before Maven use inoba/Dockerfilebuilder. It moves the pair after package installation and user setup in both runtime/server stages. The diff preserves the later runtime configuration, sobuild_bundle.shcan still useOBA_VERSION. No additional coding or automated-test requirement is stated in [#145].
Full details: Out of Scope Changes check
-
Explanation
The whole-PR diff changes only the placement of
ARG OBA_VERSIONandENV OBA_VERSIONin the four stages named by [#145]. It contains no unrelated source, dependency, configuration, or behavior changes.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.)
-
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code |
aaronbrethorst
left a comment
aaronbrethorst
left a comment
There was a problem hiding this comment.
Thanks for this. Moving the ARG/ENV OBA_VERSION pair below the apt, user-setup and connector layers in all four stages is the right call. A version bump will stop rebuilding those package layers. I checked that every use of OBA_VERSION (the mvn dependency:get/cp in the bundler builder, the mvn install and retrieve_maven_artifacts.sh in the oba builder, and the runtime ENV in both final images) still comes after the new declaration. Nothing in between depended on it. Approved on the merits.
Heads up: I just merged the Dependabot base image bumps (#146, #147, #148, #149) into main. They touched the FROM lines right next to the lines you moved, so this branch now has merge conflicts. Please merge or rebase main in and resolve them. The resolution should just be keeping the new tomcat:9.0.123-jdk11-temurin-jammy / golang:1.27.2-bookworm tags alongside your reordering. Once it's mergeable it's good to go. No re-review is needed unless the resolution changes behavior.
# Conflicts: # bundler/Dockerfile
Fixes #145. All four stages set
ARG/ENV OBA_VERSIONright before first use now, or after the package installs and user setup in the runtime and server stages, so a version bump stops rerunningapt-getandpip.build_bundle.shstill sees the variable at runtime.I couldn't build this locally (Docker wasn't running here), so CI is the real check.
Summary by CodeRabbit