Skip to content

Conversation

@pavelpatrin
Copy link
Collaborator

This pull request refactors how options are handled in the service container by changing the Option type from a function to an interface with an apply method. This enables a more extensible and object-oriented approach to option management. The changes affect both the core container logic and related tests, ensuring all option usages now call the apply method.

Core API changes:

  • Refactored the Option type in container.go from a function to an interface with an apply method, and updated all usages to call apply instead of invoking the function directly. [1] [2]
  • Added the option struct and newOption helper function to implement the new Option interface and encapsulate option logic.

Factory and service registration:

  • Updated NewFactory, NewService, and NewEntrypoint to return an Option instance using the new interface, and updated error messages for consistency. [1] [2] [3] [4] [5] [6]

Test updates:

  • Refactored all test usages of options in factory_test.go and registry_test.go to call the apply method instead of invoking the function directly, ensuring compatibility with the new interface. [1] [2] [3] [4] [5] [6] [7] [8] [9]

Signed-off-by: Pavel Patrin <ppatrin@nvidia.com>
@pavelpatrin pavelpatrin requested a review from Copilot September 27, 2025 12:42
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 pull request refactors the service container's option handling from a function-based approach to an interface-based design. The Option type is changed from a function to an interface with an apply method, enabling a more extensible and object-oriented approach to option management.

Key changes:

  • Refactored Option type from func(ctx context.Context, registry *registry) error to an interface with an apply method
  • Added option struct and newOption helper function to implement the new interface pattern
  • Updated all option usage throughout the codebase to call the apply method instead of direct function invocation

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
container.go Core refactoring of Option type to interface, updated factory/service creation functions, and added option struct implementation
factory_test.go Updated test calls to use new apply method instead of direct function invocation
registry_test.go Updated test calls to use new apply method instead of direct function invocation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@pavelpatrin pavelpatrin merged commit 2dea93f into v2-devel Sep 27, 2025
1 check passed
@pavelpatrin pavelpatrin deleted the v2-devel-options branch September 27, 2025 20:33
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.

2 participants