Closed davidkbainbridge closed 8 years ago
That is a feature that I am looking for.
@zbindenren Thanks. I have another change waiting on this one that outputs the current configuration values, which I have found useful for docker based micro services so the log contains the values which configured the service when it starts.
Hoping this gets approved and merged.
@kelseyhightower I know you are very busy, but if you could review and comment, it would be appreciated.
Hi again @davidkbainbridge!
I can spot a couple of things that should prevent merging this. I'll make some of those comments inline.
But broadly, it hardly seems right to dramatically change the structure of the code performing this package's primary job in order to support a supplementary feature. The visitor pattern as implemented here is a big increase in complexity, it'd be a much less intrusive change to just copy a little code.
@teepark:
I have to disagree wrt "copy a little code" this would essentially mean that is someone wanted to add a feature they would possibly have to edit the loop in two places and eventually things would diverge and break.
Changing to this pattern adds very little complexity, imo, and then allows the quick addition of things like this usage feature as well as another feature i am working on to "output the current config settings".
modified the code to a "visitor" pattern implementation, converted the existing
Process
method to used this pattern as well as added a visitor that can can output usage information using a Go template, including table semantics. SeeREADME.md
for example.