impleri / sow

Sow provides a CLI to Harvest using NodeJS
BSD 2-Clause "Simplified" License
26 stars 5 forks source link

add outputJson option #24

Closed lasseborly closed 6 years ago

lasseborly commented 6 years ago

This is created in reponse to #23 . I have not been doing coffeescript in a long time so more than open to feedback and suggestions. Maybe the "fork" from the standard output should not reside in the parseData function ?

Thanks for a cool, solid cli app that still runs 4 years after creation 👍

impleri commented 6 years ago

Yeah. I'm not sure that's the best place for it either. I think showCompleteLog() would be a better place for the output as it's called to render after all of the async API calls. insertEntry() can use the config value as an option for how to structure the data (e.g. raw from API or re-structured as it does already). Lastly, the CLI should be modified to allow the option (here).

I haven't used Harvest in two years, so it's crazy to see people still using this quick and dirty side project.

lasseborly commented 6 years ago

Sounds well and reasonable! Will make some modifications.

lasseborly commented 6 years ago

My main issue with not doing it in parseData is that setRootLabel() is being called and pollutes the json output. Of course I can do the config check in multiple places but as far as I can see parseData is the only place i can get away with checking once. Maybe my need for raw json output is outside this packages scope. I would just rather put effort into an already established tool than inventing something once again 👍 👎

impleri commented 6 years ago

I'm going to go ahead and merge this. I have some ideas for simplifying things which I'll do separately.