feat(codereadiness): add hook for version-based flag validation - #1819
Open
marcin11858 wants to merge 4 commits into
Open
feat(codereadiness): add hook for version-based flag validation#1819marcin11858 wants to merge 4 commits into
marcin11858 wants to merge 4 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new ChangesCodereadiness Hook
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant HookContext
participant CodeReadinessHook
participant FlagEvaluationDetails
participant VersionComparator
HookContext->>CodeReadinessHook: after(ctx, details, hints)
CodeReadinessHook->>FlagEvaluationDetails: read minCodeVersion
CodeReadinessHook->>VersionComparator: parse and compare versions
VersionComparator-->>CodeReadinessHook: true/false or exception
CodeReadinessHook-->>HookContext: pass or GeneralError
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. Comment |
marcin11858
force-pushed
the
feature/codereadiness-hook
branch
from
June 30, 2026 13:09
59cde52 to
c8244d9
Compare
oxddr
reviewed
Jul 1, 2026
marcin11858
force-pushed
the
feature/codereadiness-hook
branch
6 times, most recently
from
July 2, 2026 13:38
3e3f136 to
07126e5
Compare
marcin11858
marked this pull request as ready for review
July 6, 2026 09:04
Signed-off-by: Todd Baert <todd.baert@dynatrace.com> Signed-off-by: Marcin Wlazły <marcinwlazly@google.com>
marcin11858
force-pushed
the
feature/codereadiness-hook
branch
from
July 6, 2026 09:13
25db8d5 to
ef5127e
Compare
github-actions
Bot
requested review from
Kavindu-Dodan,
beeme1mr,
thisthat and
toddbaert
July 6, 2026 09:13
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
hooks/codereadiness/src/main/java/dev/openfeature/contrib/hooks/codereadiness/CodeReadinessHook.java (1)
17-17: 📐 Maintainability & Code Quality | 🔵 TrivialUse the generic
Hook<T>type instead of the raw type.
CodeReadinessHookimplements rawHook, andafter(...)uses rawFlagEvaluationDetailsandMap. The OpenFeature SDKHookinterface is generic (Hook<T>, withafter(HookContext<T>, FlagEvaluationDetails<T>, Map<String, Object>)); using raw types here bypasses compiler type checking and produces unchecked-operation warnings.♻️ Suggested fix
-public class CodeReadinessHook implements Hook { +public class CodeReadinessHook implements Hook<Object> { ... - public void after(HookContext ctx, FlagEvaluationDetails details, Map hints) { + public void after(HookContext<Object> ctx, FlagEvaluationDetails<Object> details, Map<String, Object> hints) {Also applies to: 47-47
🤖 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 `@hooks/codereadiness/src/main/java/dev/openfeature/contrib/hooks/codereadiness/CodeReadinessHook.java` at line 17, CodeReadinessHook is using the raw Hook type and raw after(...) parameters, which bypasses generic type safety and triggers unchecked warnings. Update CodeReadinessHook to use the generic Hook<T> signature and make its after(...) method consistently generic with HookContext<T> and FlagEvaluationDetails<T>, matching the OpenFeature SDK interface. Ensure any related Map parameter remains properly typed so the class compiles cleanly without raw-type usage.hooks/codereadiness/src/main/java/dev/openfeature/contrib/hooks/codereadiness/SemVerComparator.java (1)
27-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
isGreaterThanOrEqualTohere The two comparisons can be collapsed tocurrentSemver.isGreaterThanOrEqualTo(minCodeVersionSemver).🤖 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 `@hooks/codereadiness/src/main/java/dev/openfeature/contrib/hooks/codereadiness/SemVerComparator.java` at line 27, The version comparison in SemVerComparator is doing two separate checks for greater-than and equality that should be combined. Update the comparison logic in the relevant semver check to use currentSemver.isGreaterThanOrEqualTo(minCodeVersionSemver) instead of chaining isGreaterThan and isEqualTo, keeping the behavior the same while simplifying the expression.
🤖 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
`@hooks/codereadiness/src/main/java/dev/openfeature/contrib/hooks/codereadiness/CodeReadinessHook.java`:
- Line 17: CodeReadinessHook is using the raw Hook type and raw after(...)
parameters, which bypasses generic type safety and triggers unchecked warnings.
Update CodeReadinessHook to use the generic Hook<T> signature and make its
after(...) method consistently generic with HookContext<T> and
FlagEvaluationDetails<T>, matching the OpenFeature SDK interface. Ensure any
related Map parameter remains properly typed so the class compiles cleanly
without raw-type usage.
In
`@hooks/codereadiness/src/main/java/dev/openfeature/contrib/hooks/codereadiness/SemVerComparator.java`:
- Line 27: The version comparison in SemVerComparator is doing two separate
checks for greater-than and equality that should be combined. Update the
comparison logic in the relevant semver check to use
currentSemver.isGreaterThanOrEqualTo(minCodeVersionSemver) instead of chaining
isGreaterThan and isEqualTo, keeping the behavior the same while simplifying the
expression.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 85e7081b-8066-4050-85d0-fab3c0af2747
📒 Files selected for processing (10)
.release-please-manifest.jsonhooks/codereadiness/README.mdhooks/codereadiness/pom.xmlhooks/codereadiness/src/main/java/dev/openfeature/contrib/hooks/codereadiness/CodeReadinessHook.javahooks/codereadiness/src/main/java/dev/openfeature/contrib/hooks/codereadiness/SemVerComparator.javahooks/codereadiness/src/main/java/dev/openfeature/contrib/hooks/codereadiness/VersionComparator.javahooks/codereadiness/src/test/java/dev/openfeature/contrib/hooks/codereadiness/CodeReadinessHookTest.javahooks/codereadiness/src/test/java/dev/openfeature/contrib/hooks/codereadiness/SemVerComparatorTest.javapom.xmlrelease-please-config.json
marcin11858
force-pushed
the
feature/codereadiness-hook
branch
from
July 6, 2026 09:18
ef5127e to
fb741fb
Compare
chrfwow
reviewed
Jul 7, 2026
marcin11858
force-pushed
the
feature/codereadiness-hook
branch
from
July 22, 2026 07:53
2a3555d to
76f03bb
Compare
chrfwow
reviewed
Jul 22, 2026
marcin11858
force-pushed
the
feature/codereadiness-hook
branch
from
July 23, 2026 07:21
63dde37 to
f48a669
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
hooks/codereadiness/src/main/java/dev/openfeature/contrib/hooks/codereadiness/SemVerComparator.java (1)
18-26: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winManual "v" prefix normalization is redundant and can misfire on "V" (capital).
org.semver4j'sSemver.parse()already accepts and strips a leading "v" prefix itself, so this normalization step is unnecessary. Worse,startsWith("v")is case-sensitive: an input like"V1.2.0"won't match, so the code prepends another"v", producing"vV1.2.0", which will fail to parse instead of succeeding.♻️ Proposed simplification
public Semver parse(String versionString) { Objects.requireNonNull(versionString, "versionString cannot be null"); - String formatted = versionString.startsWith("v") ? versionString : "v" + versionString; - Semver semver = Semver.parse(formatted); + Semver semver = Semver.parse(versionString); if (semver == null) { throw new IllegalArgumentException(String.format("invalid semver: \"%s\"", versionString)); } return semver; }Please confirm
org.semver4j:semver4j:5.8.0'sSemver.parse()behavior for "v"-prefixed strings to validate this simplification is safe.🤖 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 `@hooks/codereadiness/src/main/java/dev/openfeature/contrib/hooks/codereadiness/SemVerComparator.java` around lines 18 - 26, Remove the manual prefix normalization in SemVerComparator.parse and pass the original non-null versionString directly to Semver.parse. Preserve the existing invalid-input IllegalArgumentException and null validation, relying on org.semver4j’s parser to handle leading “v” prefixes, including capital “V” inputs.
🤖 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
`@hooks/codereadiness/src/main/java/dev/openfeature/contrib/hooks/codereadiness/SemVerComparator.java`:
- Around line 18-26: Remove the manual prefix normalization in
SemVerComparator.parse and pass the original non-null versionString directly to
Semver.parse. Preserve the existing invalid-input IllegalArgumentException and
null validation, relying on org.semver4j’s parser to handle leading “v”
prefixes, including capital “V” inputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 49221b45-8abb-470c-bac1-88ae5fe77db0
📒 Files selected for processing (10)
.release-please-manifest.jsonhooks/codereadiness/README.mdhooks/codereadiness/pom.xmlhooks/codereadiness/src/main/java/dev/openfeature/contrib/hooks/codereadiness/CodeReadinessHook.javahooks/codereadiness/src/main/java/dev/openfeature/contrib/hooks/codereadiness/SemVerComparator.javahooks/codereadiness/src/main/java/dev/openfeature/contrib/hooks/codereadiness/VersionComparator.javahooks/codereadiness/src/test/java/dev/openfeature/contrib/hooks/codereadiness/CodeReadinessHookTest.javahooks/codereadiness/src/test/java/dev/openfeature/contrib/hooks/codereadiness/SemVerComparatorTest.javapom.xmlrelease-please-config.json
🚧 Files skipped from review as they are similar to previous changes (5)
- hooks/codereadiness/pom.xml
- pom.xml
- hooks/codereadiness/README.md
- hooks/codereadiness/src/main/java/dev/openfeature/contrib/hooks/codereadiness/CodeReadinessHook.java
- release-please-config.json
marcin11858
force-pushed
the
feature/codereadiness-hook
branch
from
July 23, 2026 07:42
f48a669 to
edb4fc7
Compare
marcin11858
force-pushed
the
feature/codereadiness-hook
branch
from
July 23, 2026 07:54
edb4fc7 to
51fbe31
Compare
chrfwow
reviewed
Jul 23, 2026
chrfwow
left a comment
chrfwow
left a comment
There was a problem hiding this comment.
You will need to run the spotlessApply maven target to fix the formating issues
Introduce the codereadiness hook to control feature flag evaluation by comparing the application's current version with a required minimum version specified in the flag's metadata. If the comparator returns false the hook returns an error to trigger fallback to the default flag value. Signed-off-by: Marcin Wlazły <marcinwlazly@google.com>
marcin11858
force-pushed
the
feature/codereadiness-hook
branch
from
July 23, 2026 11:15
cc0eac3 to
e505a02
Compare
chrfwow
approved these changes
Jul 23, 2026
chrfwow
left a comment
chrfwow
left a comment
There was a problem hiding this comment.
I'll approve this because the code looks good. I have not followed the conversation about this hook in the spec repo. Please make sure you align with that outcome
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduce the codereadiness hook to control feature flag evaluation by comparing the application's current version with a required minimum version specified in the flag's metadata. If the comparator returns false the hook returns an error to trigger fallback to the default flag value.
This PR
Introduces the codereadiness hook under hooks/codereadiness to control feature flag evaluation based on the application's running code version.
How it works
minCodeVersionkey).GeneralErrorto trigger the SDK's fallback mechanism, returning the flag's default value..comparator(...)), custom metadata keys (via.metadataMinVerKey(...)), and optional strict validation (via.validationRequired(...)).Notes
CodeReadinessHookTest.javaandSemVerComparatorTest.java.README.mdwith setup, configuration examples, and dependency setup.Follow-up Tasks
None.
How to test
Run the module unit tests using Maven from the project root:
mvn test -pl hooks/codereadiness