-
Notifications
You must be signed in to change notification settings - Fork 701
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
Remove snow.DefaultContextTest
#2518
Conversation
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.
change looks fine, but compilation is broken. WIll approve once CI gets to green
@@ -96,22 +96,3 @@ type ConsensusContext struct { | |||
// True iff this chain is currently state-syncing | |||
StateSyncing utils.Atomic[bool] | |||
} | |||
|
|||
func DefaultContextTest() *Context { |
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.
❤️
snow/snowtest/snowtest.go
Outdated
func EmptyContext() *snow.Context { | ||
sk, err := bls.NewSecretKey() | ||
if err != nil { | ||
panic(err) | ||
} | ||
pk := bls.PublicFromSecretKey(sk) | ||
return &snow.Context{ | ||
NetworkID: 0, | ||
SubnetID: ids.Empty, | ||
ChainID: ids.Empty, | ||
NodeID: ids.EmptyNodeID, | ||
PublicKey: pk, | ||
Log: logging.NoLog{}, | ||
BCLookup: ids.NewAliaser(), | ||
Metrics: metrics.NewOptionalGatherer(), | ||
ChainDataDir: "", | ||
} | ||
} |
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.
I know this is what we had before... but I still hate this... Are there places that we currently use EmptyContext
that we could just be using Context
?
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.
Removed EmptyContext
altogether :)
snow.DefaultContextTest
to snowtest.EmptyContext
snow.DefaultContextTest
snow.DefaultContextTest
snow.DefaultContextTest
Why this should be merged
We should be using
snowtest.Context()
in most places.How this works
Replace
How this was tested
CI