-
Notifications
You must be signed in to change notification settings - Fork 129
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
feat(services): move registry to node-core #1945
Conversation
WalkthroughThe recent changes reflect a significant restructuring of the service management within the Changes
Sequence Diagram(s)sequenceDiagram
participant A as Client
participant B as Node Core
participant C as Service Registry
A->>B: Request Service
B->>C: Register Service
C-->>B: Confirm Registration
B-->>A: Service Response
Poem
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1945 +/- ##
==========================================
+ Coverage 21.38% 21.59% +0.20%
==========================================
Files 345 352 +7
Lines 15861 15967 +106
Branches 0 21 +21
==========================================
+ Hits 3392 3448 +56
- Misses 12354 12403 +49
- Partials 115 116 +1
|
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (1)
mod/runtime/go.sum
is excluded by!**/*.sum
Files selected for processing (10)
- .mockery.yaml (1 hunks)
- mod/node-core/go.mod (2 hunks)
- mod/node-core/pkg/builder/builder.go (1 hunks)
- mod/node-core/pkg/components/interfaces.go (1 hunks)
- mod/node-core/pkg/components/service_registry.go (1 hunks)
- mod/node-core/pkg/node/node.go (1 hunks)
- mod/node-core/pkg/services/registry/mocks/registry_option.mock.go (1 hunks)
- mod/node-core/pkg/services/registry/registry_test.go (1 hunks)
- mod/node-core/pkg/types/node.go (1 hunks)
- mod/runtime/go.mod (3 hunks)
Additional comments not posted (11)
mod/node-core/pkg/types/node.go (1)
26-26
: LGTM! Verify the new import path.The import path change is consistent with the PR objective.
Ensure that the new import path
github.com/berachain/beacon-kit/mod/node-core/pkg/services/registry
is correct and accessible.Run the following script to verify the existence of the new import path:
Verification successful
Import Path Verified: The new import path is correct and accessible.
The directory
mod/node-core/pkg/services/registry
exists, confirming that the import path is valid.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the new import path. # Test: Search for the directory. Expect: The directory should exist. fd --type d 'registry' | grep 'mod/node-core/pkg/services/registry'Length of output: 102
mod/node-core/pkg/node/node.go (1)
26-26
: LGTM! Verify the new import path.The import path change is consistent with the PR objective.
Ensure that the new import path
github.com/berachain/beacon-kit/mod/node-core/pkg/services/registry
is correct and accessible.Run the following script to verify the existence of the new import path:
Verification successful
Import Path Verified
The new import pathgit.luolix.top/berachain/beacon-kit/mod/node-core/pkg/services/registry
is correct and accessible as the directory exists in the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the new import path. # Test: Search for the directory. Expect: The directory should exist. fd --type d 'registry' | grep 'mod/node-core/pkg/services/registry'Length of output: 102
mod/node-core/pkg/services/registry/mocks/registry_option.mock.go (1)
6-6
: LGTM! Verify the new import path.The import path change is consistent with the PR objective.
Ensure that the new import path
github.com/berachain/beacon-kit/mod/node-core/pkg/services/registry
is correct and accessible.Run the following script to verify the existence of the new import path:
Verification successful
Import Path Verified Successfully
The new import path
github.com/berachain/beacon-kit/mod/node-core/pkg/services/registry
is correct and accessible. The directory exists as expected.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the new import path. # Test: Search for the directory. Expect: The directory should exist. fd --type d 'registry' | grep 'mod/node-core/pkg/services/registry'Length of output: 102
.mockery.yaml (1)
41-41
: Path change is correct.The package path has been updated to reflect the new structure. The configuration options remain unchanged, ensuring consistent behavior.
mod/node-core/pkg/services/registry/registry_test.go (1)
30-31
: Import path changes are correct.The import paths have been updated to reflect the new registry service context, ensuring that the tests align with the updated structure.
mod/node-core/pkg/builder/builder.go (1)
33-33
: Import statement change is correct.The import statement has been updated to use the new registry service, aligning with the refactoring objectives.
mod/node-core/pkg/components/service_registry.go (1)
34-34
: Import path updated forservice
package.The import path for the
service
package has been updated github.com/berachain/beacon-kit/mod/node-core/pkg/services/registry
. This change aligns with the PR's objective to move the registry tonode-core
, enhancing modularity and maintainability.mod/runtime/go.mod (1)
142-142
: Dependency ongit.luolix.top/stretchr/testify
added.The addition of
github.com/stretchr/testify
as an indirect dependency enhances the module's testing capabilities. This change suggests a focus on improving testing practices.mod/node-core/go.mod (2)
72-72
: Dependency ongit.luolix.top/stretchr/objx
added.The addition of
github.com/stretchr/objx
as an indirect dependency suggests new functionality or features that rely on this library.
217-217
: Formatting change forgit.luolix.top/stretchr/testify
.The comment indicating
github.com/stretchr/testify
as an indirect dependency has been removed. This change does not affect functionality but reflects a documentation adjustment.mod/node-core/pkg/components/interfaces.go (1)
30-30
: Verify the usage of the new service registry import.Ensure that the functionalities from the newly added
service
import are correctly utilized in the codebase.Run the following script to verify the usage of the new import:
Verification successful
The new service registry import is correctly utilized.
The
service.Registry
is actively used across multiple files, indicating that the refactoring to use the new service registry is implemented correctly. No further issues were found.
- Usage confirmed in:
mod/node-core/pkg/node/node.go
mod/node-core/pkg/builder/builder.go
- and other related files.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new service registry import. # Test: Search for the usage of the new service import. Expect: At least one usage. rg --type go 'service\.' -A 5Length of output: 31411
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation