-
Notifications
You must be signed in to change notification settings - Fork 268
turn getRetiringFederationThreshold to return an optional #3425
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
turn getRetiringFederationThreshold to return an optional #3425
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
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.
Pull request overview
This PR refactors the getRetiringFederationThreshold method to return an Optional<Integer> instead of a primitive int, improving the API design by making the absence of a retiring federation explicit rather than using a magic value (-1).
Key changes:
- Modified the return type from
inttoOptional<Integer>across the federation support interface and implementations - Updated all test cases to assert on Optional presence/absence instead of checking for magic values
- Added explicit handling in the Bridge class to maintain backward compatibility by converting empty Optional to the error code
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| FederationSupport.java | Updated interface signature and documentation to return Optional |
| FederationSupportImpl.java | Simplified implementation to return Optional directly instead of using orElseGet with error code |
| BridgeSupport.java | Updated method signature to pass through Optional |
| Bridge.java | Added conversion from Optional to error code for backward compatibility |
| FederationSupportImplTest.java | Updated test assertions to check Optional.isEmpty() instead of comparing to magic value |
| BridgeSupportTest.java | Updated mock setup and assertions to work with Optional |
| BridgeSupportIT.java | Updated integration test assertions to check Optional presence and unwrap values |
| BridgeIT.java | Updated mock setup to return Optional.of(value) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…of the FEDERATION_NON_EXISTENTs
jeremy-then
left a comment
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.
|



Description
Motivation and Context
How Has This Been Tested?
Types of changes
Checklist: