Skip to content

clean up dependencies of sdk.go #635

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

Merged
merged 9 commits into from
May 27, 2024
Merged

clean up dependencies of sdk.go #635

merged 9 commits into from
May 27, 2024

Conversation

adecaro
Copy link
Contributor

@adecaro adecaro commented May 23, 2024

No description provided.

@adecaro adecaro requested review from arner and alexandrosfilios May 23, 2024 08:50
@adecaro adecaro self-assigned this May 23, 2024
@adecaro adecaro linked an issue May 23, 2024 that may be closed by this pull request
@adecaro adecaro linked an issue May 24, 2024 that may be closed by this pull request
@@ -140,6 +141,73 @@ func (p *ManagementServiceProvider) managementService(aNew bool, opts ...Service
return ms, nil
}

func (p *ManagementServiceProvider) normalize(opt *ServiceOptions) (*ServiceOptions, error) {
// lookup configurations
configs, err := p.tmsProvider.Configs()
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's better to create another struct that implements the normaliser interface and pass the slice of normalisers.

Copy link
Contributor

@arner arner left a comment

Choose a reason for hiding this comment

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

LGTM!

token/sdk/sdk.go Outdated
@@ -205,9 +179,18 @@ func (p *SDK) Start(ctx context.Context) error {
logger.Infof("start token management service [%s]...", tmsID)

// connect network
if err := p.postInitializer.ConnectNetwork(tmsID.Network, tmsID.Channel, tmsID.Namespace); err != nil {
net := network.GetInstance(p.registry, tmsID.Network, tmsID.Channel)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are in the sdk and we have access to the network provider, the tms provider etc, maybe we could use those instead of calling the registry

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

New(tmsID token3.TMSID) (F, error)
}

type AcceptTxInDBFilterProvider struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a comment to describe more or less what this filter is used for

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@alexandrosfilios alexandrosfilios left a comment

Choose a reason for hiding this comment

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

LGTM

adecaro added 2 commits May 27, 2024 15:48
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
@adecaro adecaro merged commit df071e8 into main May 27, 2024
38 checks passed
@adecaro adecaro deleted the f-522 branch May 27, 2024 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clean up dependencies of sdk.go Orion network service launched even no orion network config
3 participants