-
Notifications
You must be signed in to change notification settings - Fork 61
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
breaking(objectstore): rename object-store
command to kv-store
#904
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.
LGTM. Only a couple of minor change requests.
Co-authored-by: Mark McDonnell <Integralist@users.noreply.github.com>
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.
LGTM. Will merge once CI is finished (and passing).
object-store
command to kv-store
@@ -839,7 +839,7 @@ func pkgUpload(spinner text.Spinner, client api.Interface, serviceID string, ver | |||
return spinner.Stop() | |||
} | |||
|
|||
func constructSetupObjects( | |||
func constructSetupKVs( |
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.
Wondering if this should have been left as constructSetupObjects
? Since setup.Backends
, for example, aren't key-value stores. Working through merge conflicts in #769 and noticed this.
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.
@awilliams-fastly correct! I'm actually fixing this at the moment 🙂
We are renaming object store to kv store so doing the needed changes here.
Blocked by: fastly/go-fastly#422