-
Notifications
You must be signed in to change notification settings - Fork 191
Conversation
@@ -8,10 +8,12 @@ import ( | |||
"github.com/docker/libcompose/project" | |||
) | |||
|
|||
type ProjectFactory struct { | |||
// DockerProjectFactory is a struct that hold the app.ProjectFactory implementation. | |||
type DockerProjectFactory 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.
I renamed that as it felt a little weird to have the exact same name for the interface and the struct implementing it. If it breaks outside uses of the lib, tell me, I'll revert that change.
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.
It's pretty common in go to do this because any time you reference the type you also include package. So calling code would look like dockerApp.ProjectFactory
. Making the type have the package name in it then makes it redundant dockerApp.DockerProjectFactory
.
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 didn't see it like that. I'll revert this one, agre with you, it makes it redundant.
@ibuildthecloud Updated according to your comments 馃槈. |
packages: cli/app, cli/command, cli/docker/app, cli/logger, cli/main Signed-off-by: Vincent Demeester <vincent@sbr.pm>
LGTM 馃憤 |
LGTM |
馃帀 馃槈 |
Updated packages:
cli/app
,cli/command
,cli/docker/app
,cli/logger
,cli/main
.Wating for #21 to be merged but that opens it to reviews already 馃槈.
馃惛
Signed-off-by: Vincent Demeester vincent@sbr.pm