pass the network driver to the constructor #714
Conversation
ci/fsc-fts-update.sh
Outdated
| @@ -0,0 +1,39 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
I believe this script belongs to the fts repo
There was a problem hiding this comment.
I really meant to have a script that updates FTS with the current FSC commit.
There was a problem hiding this comment.
I understand. However, I think this script should move to the FTS repo as it creates a PR on the FTS repo.
Such a script could support multiple strategies:
- Get the commit hash from any local git clone / working branch.
- Get the lastest commit hash from main
- Provide the commit hash as a parameter to the script and update all go.mod files inside the project.
I believe the right way would be to use dependabot and schedule a daily check if the main branch on FSC has something new. As dependabot only "detects" versioned updated (not individual commits) it won't work like that. You can create a GH action that is fired once per day to check if there is a new commit on FSC main branch and then create a PR.
There was a problem hiding this comment.
okay, I'll remove the script.
| committerService := p.newCommitter( | ||
| nw.ConfigService(), | ||
| committerService, err := p.newCommitter( | ||
| nw, |
There was a problem hiding this comment.
Why do we pass the whole nw? We just want a few services, right?
There was a problem hiding this comment.
yeah, but we don't know before hand which services we might need. These constructors are called during the invocation of this function type NewChannelFunc = func(network driver.FabricNetworkService, name string, quiet bool) (driver.Channel, error). It seems fair to me that also the constructors get network. No?
| @@ -133,6 +137,16 @@ func (o *Service) SetConsensusType(consensusType ConsensusType) error { | |||
| return nil | |||
| } | |||
|
|
|||
| func (f *Service) SetConfigOrderers(o channelconfig.Orderer, orderers []*grpc.ConnectionConfig) error { | |||
There was a problem hiding this comment.
We pass o that already contains Consenters and orderers. Do we need both? We could only pass ordererType.
There was a problem hiding this comment.
good point, we can simplify this.
| type LedgerConstructor func( | ||
| channelName string, | ||
| nw driver.FabricNetworkService, |
There was a problem hiding this comment.
I don't know if it's a good idea to replace the actual dependencies by a struct that contains these and many more. This creates a tighter coupling and looser cohesion.
There was a problem hiding this comment.
yeah, but we don't know before hand which services we might need. These constructors are called during the invocation of this function type NewChannelFunc = func(network driver.FabricNetworkService, name string, quiet bool) (driver.Channel, error). It seems fair to me that also the constructors get network. No?
alexandrosfilios
force-pushed
the
f-ndc
branch
4 times, most recently
from
835f0e4 to
3a68044
Compare
Signed-off-by: Alexandros Filios <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
alexandrosfilios
force-pushed
the
f-ndc
branch
from
3a68044 to
b19b5fa
Compare
This PR simplify the constructors used to create a new channel. It also replaces certain dependencies with interfaces and not structs.
This PR is tested against FTS in hyperledger-labs/fabric-token-sdk#804