Skip to content

Hardcode VERSION to oadp-dev for standardised version reporting#188

Merged
Joeavaikath merged 1 commit into
migtools:oadp-devfrom
Joeavaikath:hardcode-version-string
May 8, 2026
Merged

Hardcode VERSION to oadp-dev for standardised version reporting#188
Joeavaikath merged 1 commit into
migtools:oadp-devfrom
Joeavaikath:hardcode-version-string

Conversation

@Joeavaikath
Copy link
Copy Markdown
Contributor

@Joeavaikath Joeavaikath commented May 7, 2026

Why the changes were made

Previously, VERSION was derived from git describe --tags --always --dirty,
which produced opaque commit hashes (e.g., c548518-dirty) as the version
string. This made oc oadp version output meaningless to users.

This PR hardcodes the version to oadp-dev in both the Makefile and
Containerfile.download so that users get a clear, recognisable version
string that maps to the branch the binary was built from. When a release
branch is cut (e.g., oadp-1.6), this value is updated as part of the
branch cut process. The git SHA is still injected separately for exact
commit identification.

How to test the changes made

  1. Build locally via make build and run ./kubectl-oadp version
    client version should show oadp-dev.
  2. Build via make install and run oc oadp version — same result.
  3. Build the container image with podman build -f Containerfile.download .
    and verify the embedded binaries report oadp-dev.

Summary by CodeRabbit

  • Chores
    • Updated default version identifier in build configurations from dev to oadp-dev to ensure consistent versioning across release artifacts and container builds.

Previously VERSION was derived from git tags/describe, which produced
opaque commit hashes. Hardcoding to the branch name (oadp-dev) gives
users a meaningful version string via `oc oadp version`. Release
branches update this value when cut (e.g., oadp-1.6).

Signed-off-by: Joseph <jvaikath@redhat.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 7, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR standardizes version defaults across build configurations. Containerfile.download and Makefile now both default to the constant oadp-dev instead of deriving versions from git tags or using dev as a fallback.

Changes

Version Default Unification

Layer / File(s) Summary
Build Version Defaults
Containerfile.download, Makefile
ARG OADP_VERSION defaults to oadp-dev in Containerfile and VERSION variable defaults to oadp-dev in Makefile, replacing git-describe derivation and fallback logic.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • migtools/oadp-cli#186: Both PRs modify how the build-time OADP version is provided to container builds (changing default/version source in Dockerfile/Containerfile/Makefile).
  • migtools/oadp-cli#184: Both PRs address the same code-level concern of using OADP_VERSION instead of VERSION for build-time version injection (build arg/ldflags).
  • migtools/oadp-cli#180: Both PRs touch the same build/versioning inputs—changing default OADP/VERSION values while also wiring VERSION into container build arguments and LDFLAGS.

Suggested labels

lgtm

Suggested reviewers

  • kaovilai
  • weshayutin
  • mpryc

Poem

🐰 A version once wild, now tamed to one name,
"oadp-dev" steady, no git-describe game,
Containerfile, Makefile in harmony sing,
One constant default—what simplicity brings! 📦✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change—hardcoding VERSION to oadp-dev for standardised version reporting—which aligns with the core objective of the changeset.
Description check ✅ Passed The PR description includes both required sections with comprehensive explanations: 'Why the changes were made' explains the motivation and benefit, and 'How to test the changes made' provides clear testing instructions with specific commands and expected outcomes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
Makefile (1)

8-8: Ensure release pipeline explicitly passes VERSION.

Since VERSION ?= oadp-dev is now a static string (not derived from git tags), any CI/CD release job that does not explicitly set VERSION=oadp-X.Y will silently produce archives, checksums, and krew manifests stamped oadp-dev. A guard or validation step in the release workflow (e.g., asserting VERSION != oadp-dev before publishing) would prevent accidental dev-labeled releases.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Makefile` at line 8, The Makefile currently defaults VERSION ?= oadp-dev
which can silently produce dev-stamped artifacts; add a guard that fails the
build when VERSION remains the default (e.g., add a validate step or a top-level
conditional check that errors if VERSION == oadp-dev) and ensure release targets
(e.g., your existing release/build targets) depend on that validation, or
alternatively add a CI workflow assertion that explicitly requires VERSION to be
set (assert VERSION != oadp-dev) before publishing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@Makefile`:
- Line 8: The Makefile currently defaults VERSION ?= oadp-dev which can silently
produce dev-stamped artifacts; add a guard that fails the build when VERSION
remains the default (e.g., add a validate step or a top-level conditional check
that errors if VERSION == oadp-dev) and ensure release targets (e.g., your
existing release/build targets) depend on that validation, or alternatively add
a CI workflow assertion that explicitly requires VERSION to be set (assert
VERSION != oadp-dev) before publishing.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 34d1d68c-f3e0-44eb-b2bb-84969b2c2cf4

📥 Commits

Reviewing files that changed from the base of the PR and between c548518 and 5d08203.

📒 Files selected for processing (2)
  • Containerfile.download
  • Makefile

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 7, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Joeavaikath, shubham-pampattiwar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [Joeavaikath,shubham-pampattiwar]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Copy Markdown
Member

@kaovilai kaovilai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@Joeavaikath Joeavaikath merged commit 21c2e9c into migtools:oadp-dev May 8, 2026
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants