marathon-consul icon indicating copy to clipboard operation
marathon-consul copied to clipboard

Remove ogier/pflag

Open janisz opened this issue 7 years ago • 8 comments

We are using ogier/pflag because it was introduced in original CiscoCloud project. pflag looks like abandoned (last commit over a year ago). We should switch to official golang flags. This change could be not backward compatible because ogier uses double dash -- while golang prefers - (see this comment)

janisz avatar Mar 02 '17 10:03 janisz

Is it possible to switch to combo cobra / viper? It's really powerful, simple to use and with a big community.

guilhem avatar Mar 06 '17 12:03 guilhem

not to mention that, opposite to #195, we can be backward compatible

guilhem avatar Mar 06 '17 12:03 guilhem

I made a small comparison between viper and flag. We try to minimize dependencies we are using in the code. Viper looks nice but we don't need most of it's features Changing from ogier to viper only to have live and supported project as a dependency is pointless. I hope reducing one dash (-) from flags won't be change that will cause an outage. We can remove ogier after we introduce SSE. So we will make two changes in config at once.

feature viper flag required by marathon-consul
setting defaults
reading from JSON, TOML, YAML, HCL, and Java properties config files
live watching and re-reading of config files (optional) ✗⁺
reading from environment variables ✗⁺
reading from remote config systems (etcd or Consul), and watching changes ✗⁺
reading from command line flags
reading from buffer
setting explicit values
"readability" of code (by removing the "config" object)
backward compatible
auto-generated documentation
simple usage

⁺ nobody requested it

janisz avatar Mar 06 '17 13:03 janisz

So because nobody asks for a feature (which is just best practice) it's not a feature to have?
Strange mindset. You also forgot about:

  • "readability" of code (by removing the "config" object)
  • backward compatible.

One big point I mention was the link with cobra which is really helpful for an app:

  • auto-generated documentation
  • simple usage
  • ...

Of course, everything is optional and an application can live without it... but it's just today right here, we have vendoring, why not using it "for free"?

guilhem avatar Mar 06 '17 14:03 guilhem

Personally, I found https://github.com/jawher/mow.cli which works best for me. You could consider it if you like.

kamilchm avatar Mar 06 '17 15:03 kamilchm

Thanks for response. Indeed I forgot about some features of cobra/viper that we might want. I'm not familiar with this tools so I'm not a fan of it. From my perspective ogier does nothing to help us and that's why I want to remove it. Adding new dependency (outside of stdlib) means somebody need to watch it and update when there are bugs to prevent users of marathon-consul to be hit by an error in our dependencies.

On the other hand, some features would be really nice:

  • configuration in YAML will allow having comments in config
  • removing config.Config struct
  • reading configuration from Consul
  • auto generated usage and readme

janisz avatar Mar 06 '17 15:03 janisz

Combo Viper & Cobra looks nice :)

wendigo avatar Mar 06 '17 17:03 wendigo

Anny update on this? Does anybody want to prepare PR with better configs handling?

janisz avatar Mar 30 '17 15:03 janisz