mdsol / dice_bag

DiceBag is a library of rake tasks for configuring web apps in the style of The Twelve-Factor App.
MIT License
19 stars 4 forks source link

add generate_from_gems command #79

Closed jfeltesse-mdsol closed 7 years ago

jfeltesse-mdsol commented 7 years ago

@JordiPolo still need to polish things, add specs and adhoc on ruby 1.9 but that's essentially it. Allows you to do:

rake config:generate_from_gems some_gem_name another_gem_name

DICEBAG_FORCE=1 rake config:generate_from_gems some_gem_name another_gem_name
jcarres-mdsol commented 7 years ago

could somehow only add one new command instead of two? We already have 5 and the more , the more confusing it will be to distinguish them

jfeltesse-mdsol commented 7 years ago

I followed the pattern used by generate_all / generate_all:force, not so familiar with rake tasks args and such, will gladly make it one command if it's possible but how?

ssteeg-mdsol commented 7 years ago

Maybe just add a third parameter to the command? rake config:generate_from_gems[some_gem_name,another_gem_name,force] and I believe you can make the third parameter optional, by handling the not present case in your rake task as force=false

As a reference, I bookmarked http://cobwwweb.com/4-ways-to-pass-arguments-to-a-rake-task for dealing with rake task args.

jfeltesse-mdsol commented 7 years ago

Switched to using ARGV as described in the article @ssteeg-mdsol linked to. My initial adhocing proved to be poor as I discovered you cannot use commas in rake task args... 😥

I see there are 0 specs for core functionality, do I get away with this too and do I need to start adding some before this deserves a merge?

jfeltesse-mdsol commented 7 years ago

(see the updated description, I also used an env var to enable force instead of copy/pasting the task)

ssteeg-mdsol commented 7 years ago

Seems okay to me. Specs are always nice to have :trollface:

ssteeg-mdsol commented 7 years ago

Description still has WIP. Is that true?

jfeltesse-mdsol commented 7 years ago

@ssteeg-mdsol removed now. Depends on whether the J to the P is satisfied with a spec-less PR 😉

jcarres-mdsol commented 7 years ago

Spec-less FTW, only one comment

jfeltesse-mdsol commented 7 years ago

@JordiPolo I made that call to be next if !gem_names.empty? && !checker_within_given_gems?(checker, gem_names)

Reads OK to me, let me know if it feels strange to you (such cases can be highly subjective)

cabbott commented 7 years ago

I'm good for merging today if @jcarres-mdsol has no objections

jcarres-mdsol commented 7 years ago

looks good!

jcarres-mdsol commented 7 years ago

https://rubygems.org/gems/dice_bag/versions/1.2.0