Skip to content
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 MongoSnap fields to JujuDBSnap to match snap name. #11475

Merged
merged 1 commit into from Apr 21, 2020

Conversation

howbazaar
Copy link
Contributor

In order to avoid future confusion, we want to have the controller config
named 'juju-db-snap-channel' because the channel is for the juju-db snap.

This also means should we change what the underlying db is, we don't have
to change the config name. Although perhaps that is wishful thinking.

In order to avoid future confusion, we want to have the controller config
named 'juju-db-snap-channel' because the channel is for the juju-db snap.

This also means should we change what the underlying db is, we don't have
to change the config name. Although perhaps that is wishful thinking.
Copy link
Member

@wallyworld wallyworld left a comment

Choose a reason for hiding this comment

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

Just a desire to use jujudb-snap-channel as it looks better IMHO

@@ -60,7 +60,7 @@ type format_2_0Serialization struct {
SystemIdentity string `yaml:"systemidentity,omitempty"`
MongoVersion string `yaml:"mongoversion,omitempty"`
MongoMemoryProfile string `yaml:"mongomemoryprofile,omitempty"`
MongoSnapChannel string `yaml:"mongosnapchannel,omitempty"`
JujuDBSnapChannel string `yaml:"juju-db-snap-channel,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps can we make this jujudb-snap-channel

The other fields don't have hyphens (they should) and reducing the number we use in this new field looks a little tidier IMO

// snaps for focal or later. The value is ignored for older releases.
MongoSnapChannel = "mongo-snap-channel"
JujuDBSnapChannel = "juju-db-snap-channel"
Copy link
Member

Choose a reason for hiding this comment

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

and here wrt other suggestion

@@ -139,7 +139,7 @@ func (s *AgentConfigUpdaterSuite) TestCentralHubMissing(c *gc.C) {
*result = params.ControllerConfigResult{
Config: map[string]interface{}{
"mongo-memory-profile": "default",
"mongo-snap-channel": controller.DefaultMongoSnapChannel,
"juju-db-snap-channel": controller.DefaultJujuDBSnapChannel,
Copy link
Member

Choose a reason for hiding this comment

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

ditto

@howbazaar
Copy link
Contributor Author

$$merge$$

@jujubot jujubot merged commit 2eb0b96 into juju:develop Apr 21, 2020
@achilleasa
Copy link
Contributor

@howbazaar I actually considered renaming this as you did here but decided against it in the end given that the other mongo options (e.g. the memory-profile one) start with mongo-. Should we also change any var/const names in the code to align with this change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants