refactoring mate config #68
Conversation
ideahitme
commented
Jan 17, 2017
•
ideahitme
commented
- Create config.go to store all mate configurations
- Remove flags initialisation from all internal packages (this is generally a bad practice to do so)
- Make internal packages more independent and actually "invocable"
|
@linki I would like to hear your opinion on this :) |
|
|
||
| KubeServer *url.URL | ||
| KubeFormat string | ||
| TrackNodePorts bool |
There was a problem hiding this comment.
how about - for consistency with the flags - to prefix with kubernetes or change the flags to kube? (the latter would require a version bump)
There was a problem hiding this comment.
+1 for kubernetes prefix :)
| if err != nil { | ||
| log.Fatalf("Error creating producer: %v", err) | ||
| } | ||
|
|
||
| c, err := consumers.NewSynced(params.consumer) | ||
| c, err := newSyncConsumer(cfg) |
| case "aws": | ||
| consumer, err = consumers.NewAWSConsumer(cfg.AWSRecordGroupID) | ||
| case "stdout": | ||
| consumer, err = consumers.NewStdout() |
There was a problem hiding this comment.
if you want we can also change these functions to:
consumers.NewGoogleCloudDNSConsumerconsumers.NewAWSRoute53Consumerconsumers.NewStdoutConsumerconsumers.NewSynchronizedConsumer
to have unified and unambiguous naming.
| } | ||
| return producers.NewFake(fakeConfig) | ||
| case "null": | ||
| return producers.NewNull() |
There was a problem hiding this comment.
let's change this to
producers.NewKubernetesProducerproducers.NewFakeProducerproducers.NewNullProducer
| endpoint.Hostname = fmt.Sprintf("%s.%s", randomString(6), a.targetDomain) | ||
| case fixedMode: | ||
| case "fixed": |
There was a problem hiding this comment.
you still have those modes defined as const up there. We should use them. Also I think it's a good idea.
|
I think this is a very valuable PR for mate 👍 I would be glad if you can sneak in the changes I proposed as well. |
|
@linki thanks for valuable comments :) please take a look again |
|
awesome @ideahitme |
|
one last thing... |
| if err != nil { | ||
| return nil, err | ||
| } | ||
| func NewSynchronizedConsumer(consumer Consumer) (Consumer, error) { | ||
| return &SyncedConsumer{Consumer: consumer}, nil |
There was a problem hiding this comment.
let's also change this struct name
|
👍 |
