mongo: move MongoInfo type #647
Conversation
| // MongoInfo encapsulates information about cluster of | ||
| // servers holding juju state and can be used to make a | ||
| // connection to that cluster. | ||
| type MongoInfo struct { |
There was a problem hiding this comment.
I'm wondering if you'd consider renaming the struct from MongoInfo to ConnectionInfo, so the other sites that use it will have mongo.ConnectionInfo. Thoughts?
There was a problem hiding this comment.
SGTM. But not just yet, we can repaint the bike shed once I've finished
moving things around.
On Mon, Sep 1, 2014 at 6:11 PM, Tim Penhey [email protected] wrote:
In environmentserver/authentication/authentication.go:
@@ -14,20 +14,6 @@ import (
apiprovisioner "github.com/juju/juju/state/api/provisioner"
)-// MongoInfo encapsulates information about cluster of
-// servers holding juju state and can be used to make a
-// connection to that cluster.
-type MongoInfo struct {I'm wondering if you'd consider renaming the struct from MongoInfo to
ConnectionInfo, so the other sites that use it will have
mongo.ConnectionInfo. Thoughts?—
Reply to this email directly or view it on GitHub
https://github.com/juju/juju/pull/647/files#r16944216.
There was a problem hiding this comment.
Also, MongoInfo embeds a mongo.Info, so renaming the outer type doesn't
solve the problem of mongo.MongoInfo.Info. O-O
On Mon, Sep 1, 2014 at 6:29 PM, David Cheney [email protected]
wrote:
SGTM. But not just yet, we can repaint the bike shed once I've finished
moving things around.On Mon, Sep 1, 2014 at 6:11 PM, Tim Penhey [email protected]
wrote:In environmentserver/authentication/authentication.go:
@@ -14,20 +14,6 @@ import (
apiprovisioner "github.com/juju/juju/state/api/provisioner"
)-// MongoInfo encapsulates information about cluster of
-// servers holding juju state and can be used to make a
-// connection to that cluster.
-type MongoInfo struct {I'm wondering if you'd consider renaming the struct from MongoInfo to
ConnectionInfo, so the other sites that use it will have
mongo.ConnectionInfo. Thoughts?—
Reply to this email directly or view it on GitHub
https://github.com/juju/juju/pull/647/files#r16944216.
There was a problem hiding this comment.
On 01/09/14 20:30, Dave Cheney wrote:
In environmentserver/authentication/authentication.go:
@@ -14,20 +14,6 @@ import (
apiprovisioner "github.com/juju/juju/state/api/provisioner"
)-// MongoInfo encapsulates information about cluster of
-// servers holding juju state and can be used to make a
-// connection to that cluster.
-type MongoInfo struct {Also, MongoInfo embeds a mongo.Info, so renaming the outer type doesn't
solve the problem ofmongo.MongoInfo.Info. O-O
Fair enough. LGTM
There was a problem hiding this comment.
Thanks.
I am 1/2 hoping that out of this set of changes we'll find we have 3 or so
slightly different data types that can be rolled into one.
On Mon, Sep 1, 2014 at 6:35 PM, Tim Penhey [email protected] wrote:
In environmentserver/authentication/authentication.go:
@@ -14,20 +14,6 @@ import (
apiprovisioner "github.com/juju/juju/state/api/provisioner"
)-// MongoInfo encapsulates information about cluster of
-// servers holding juju state and can be used to make a
-// connection to that cluster.
-type MongoInfo struct {On 01/09/14 20:30, Dave Cheney wrote: In
environmentserver/authentication/authentication.go: > @@ -14,20 +14,6 @@
import ( > apiprovisioner "github.com/juju/juju/state/api/provisioner" >
) > > -// MongoInfo encapsulates information about cluster of > -// servers
holding juju state and can be used to make a > -// connection to that
cluster. > -type MongoInfo struct { Also, MongoInfo embeds a mongo.Info, so
renaming the outer type doesn't solve the problem ofmongo.MongoInfo.Info.
O-O
Fair enough. LGTM—
Reply to this email directly or view it on GitHub
https://github.com/juju/juju/pull/647/files#r16945052.
There was a problem hiding this comment.
On Mon, Sep 1, 2014 at 6:37 PM, David Cheney [email protected]
wrote:
Thanks.
I am 1/2 hoping that out of this set of changes we'll find we have 3 or so
slightly different data types that can be rolled into one.On Mon, Sep 1, 2014 at 6:35 PM, Tim Penhey [email protected]
wrote:In environmentserver/authentication/authentication.go:
@@ -14,20 +14,6 @@ import (
apiprovisioner "github.com/juju/juju/state/api/provisioner"
)-// MongoInfo encapsulates information about cluster of
-// servers holding juju state and can be used to make a
-// connection to that cluster.
-type MongoInfo struct {On 01/09/14 20:30, Dave Cheney wrote: In
environmentserver/authentication/authentication.go: > @@ -14,20 +14,6 @@
import ( > apiprovisioner "github.com/juju/juju/state/api/provisioner" >
) > > -// MongoInfo encapsulates information about cluster of > -// servers
holding juju state and can be used to make a > -// connection to that
cluster. > -type MongoInfo struct { Also, MongoInfo embeds a mongo.Info, so
renaming the outer type doesn't solve the problem ofmongo.MongoInfo.Info. O-O
Fair enough. LGTM—
Reply to this email directly or view it on GitHub
https://github.com/juju/juju/pull/647/files#r16945052.
|
|
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
mongo: move MongoInfo type This PR moves the `MongoInfo` type from the `environmentserver/authentication` package to the `mongo` package. This change is necessary to unblock a larger change which was hitting a circular dependency. With that said, as this change consists of _removing_ the import of the `environmentserver/authentication` from most packages (and renaming the type), it seems clear in hindsight that the `MongoInfo` type belongs in the `mongo` package.
This PR moves the
MongoInfotype from theenvironmentserver/authenticationpackage to themongopackage.This change is necessary to unblock a larger change which was hitting a circular dependency.
With that said, as this change consists of removing the import of the
environmentserver/authenticationfrom most packages (and renaming the type), it seems clear in hindsight that theMongoInfotype belongs in themongopackage.