-
Notifications
You must be signed in to change notification settings - Fork 479
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
Rename the StateStore interface #70
Conversation
state/cosmosdb/cosmosdb.go
Outdated
// StateStore is a CosmosDB state store | ||
type StateStore struct { | ||
// CosmosDB is a CosmosDB state store | ||
type CosmosDB struct { |
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.
package name is cosmosdb and you renamed StateStore to CosmosDB. cosmosdb.CosmosDB doesn't look good to me. Do you have the better idea for this ? maybe StateStore would be better.. cc/ @yaron2
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.
Oh i take a look on other implementation (eg etcd), and would like to make it consistent. Will be easier for consumer
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.
@sayboras , can we just have the changes for renaming the statestore interface only ?
The reason is that we have issue open to rename the package names , in that effort we will taking the name changes in specific PR.
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.
@shalabhms good point. once interface is renamed, we have to change in dapr as well. for renaming the actual struct, it would be great to create the separate pr per component implementation.
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.
@shalabhms @youngbupark noted and thanks guys, I have updated PR, kindly help to take another look
Description
Fixes #67
Issue reference
As mentioned in #67, we would like to rename interface
state.StateStore
to juststate.Store
.I also noticed that there is inconsistency in naming for different store implementation such as
cosmodb
andredis
compared to others (e.g. etcd, cassandra, consul etc). Hence, I have done some refactor as well. This might be breaking change, so would love to hear your inputs.Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: