Skip to content

Conversation

@julianlen
Copy link
Contributor

Description

Motivation and Context

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • Tests for the changes have been added (for bug fixes / features)
  • Requires Activation Code (Hard Fork)
  • Other information:

@julianlen julianlen requested a review from a team as a code owner December 19, 2025 14:40
@github-actions
Copy link

github-actions bot commented Dec 19, 2025

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@julianlen julianlen force-pushed the getRetiringFederationSize-optional branch from e5368e8 to ff847bf Compare December 19, 2025 14:43
@julianlen julianlen requested a review from Copilot December 19, 2025 14:43
Copy link

Copilot AI left a 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 getRetiringFederationSize() method to return an Optional<Integer> instead of a primitive int, eliminating the need to use magic number -1 to represent a non-existent retiring federation. The change improves type safety and makes the API more explicit about the possibility of absent values.

Key Changes:

  • Modified the return type of getRetiringFederationSize() from int to Optional<Integer> across the interface and implementation layers
  • Updated all test cases to verify optional emptiness instead of checking for the magic number response code
  • Added proper unwrapping of the Optional at the Bridge layer boundary, converting back to the magic number for backward compatibility

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
FederationSupport.java Updated interface signature to return Optional<Integer>
FederationSupportImpl.java Simplified implementation to return Optional directly instead of falling back to error code
BridgeSupport.java Updated return type to propagate Optional<Integer>
Bridge.java Added unwrapping logic with fallback to FEDERATION_NON_EXISTENT code for backward compatibility
FederationSupportImplTest.java Updated assertions to check for empty Optional instead of error code
BridgeSupportTest.java Updated mock setup to return Optional
BridgeSupportIT.java Updated integration test assertions to verify Optional presence/absence
BridgeIT.java Updated mock to return Optional.of(1234)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@julianlen julianlen force-pushed the getRetiringFederationSize-optional branch 2 times, most recently from 86510d2 to 05a40f5 Compare December 19, 2025 18:24
Copy link
Contributor

@jeremy-then jeremy-then left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor

@julia-zack julia-zack left a comment

Choose a reason for hiding this comment

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

left one super minor comment

@julianlen julianlen force-pushed the getRetiringFederationSize-optional branch from d2eaf0e to c196617 Compare December 19, 2025 19:37
@sonarqubecloud
Copy link

@julianlen julianlen merged commit a98a45b into mtmu-integration Dec 22, 2025
9 checks passed
@marcos-iov marcos-iov deleted the getRetiringFederationSize-optional branch December 22, 2025 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants