Skip to content
This repository was archived by the owner on Feb 29, 2024. It is now read-only.

Conversation

@xredo
Copy link
Member

@xredo xredo commented Apr 6, 2017

Closes #27

@xredo xredo requested a review from fjsanpedro April 6, 2017 09:42

def simple_form_locale_sample
File.join(File.dirname(__FILE__), 'simple_form', 'en.yml')
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not a big deal but, instead having the same behavior duplicated why not gather it:

`
def run_generators
case ask_framework_for_forms
when 'marsman'
@template.initializer 'simple_form.rb', simple_form_marsman_template
when 'bootstrap'
@template.generate 'simple_form:install --bootstrap'
else
@template.generate 'simple_form:install'
end
end

def add_sample_i18n
  @template.run 'rm config/locales/simple_form.en.yml'
  @template.append_to_file 'config/locales/en.yml', simple_form_locale_sample
end

def ask_framework_for_forms
  @template.ask('What framework do you want to use for your forms?',
                limited_to: %w(marsman bootstrap default))
end

def simple_form_marsman_template
  simple_form_sample('marsmab.rb')
end

def simple_form_locale_sample
  simple_form_sample('en.yml'')
end

def simple_form_sample(filename)
  File.read(File.join(File.dirname(__FILE__), 'simple_form', filename))
end

`

My 2 cents

@xredo
Copy link
Member Author

xredo commented Apr 6, 2017

What do you think @fjsanpedro? I'm only using one method, instead of 2, but having the behaviour unified was necessary, as you suggested!

@fjsanpedro
Copy link
Contributor

Great, ready to 🚀

@xredo xredo merged commit b46a191 into master Apr 6, 2017
@xredo xredo removed the in progress label Apr 6, 2017
@xredo xredo deleted the feature/#27-simple-form-marsman-boostrap branch April 6, 2017 13:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants