Change the flags that enable/disable a component use 'Enabled' for co… #60
Conversation
| @@ -258,6 +258,7 @@ var defaultConfig = Config{ | |||
| }, | |||
| }, | |||
| Websocket: Websocket{ | |||
| Enabled: true, | |||
| URLPath: "api/v2/device", | |||
There was a problem hiding this comment.
added a new default if we wanted to ws to be enable by default
#60 (review)
There was a problem hiding this comment.
this is fine but does disabled make more sense because default bool is false? I missed this pattern so I can switch the mocktr181 to use disabled if that's better
There was a problem hiding this comment.
fyi @schmidtw in case you didn't want this behavior
There was a problem hiding this comment.
because of the power of goschtalt, either one is fine and both works for me
There was a problem hiding this comment.
Hmm - the question I have is:
If this defaults to on, will the default configuration actually work for any service?
If you need to provide some configuration to make ti work, I think defaulting it to off is ok. If it will work just fine without any non-default values, then I agree that Disabled would make more sense.
There was a problem hiding this comment.
what do you think @johnabass ? nevermind
There was a problem hiding this comment.
@denopink and I chatted & while well intended, I'm not merging this PR for a few reasons:
- When we enable QUIC as a protocol, we'll know better how we want to refactor the config & yes, it will be a breaking change, but that's probably better than guessing about several things and being wrong & then still have a breaking change later.
- This doesn't help us reach our initial goal/deadline.
There was a problem hiding this comment.
Thanks for the feedback!
…nsistency.