Skip to content

Loading…

Update current gem defaults #139

Closed
wants to merge 2 commits into from

6 participants

@croaky
thoughtbot, inc. member

Provide fewer options and more opinions:

  • Add assets group to Gemfile.
  • Add jQuery to Gemfile.
  • Add Postgres to Gemfile.
  • Add Rails to Gemfile.
  • Add Strong Parameters to Gemfile.
  • Alphabetize existing gems.
  • Make Capybara Webkit a default.
  • Remove Clearance.
  • Remove Flutie.
  • Remove Paperclip.
  • Replace Formtastic with Simple Form.
  • Specify minimum version of Capybara Webkit.
  • Specify minimum version of Rails.
@croaky croaky Update current gem defaults
Provide fewer options and more opinions:

* Add assets group to Gemfile.
* Add jQuery to Gemfile.
* Add Postgres to Gemfile.
* Add Rails to Gemfile.
* Add Strong Parameters to Gemfile.
* Alphabetize existing gems.
* Make Capybara Webkit a default.
* Remove Clearance.
* Remove Flutie.
* Remove Paperclip.
* Replace Formtastic with Simple Form.
* Specify minimum version of Capybara Webkit.
* Specify minimum version of Rails.
465edd4
@jferris jferris commented on the diff
templates/Gemfile_clean
@@ -1,11 +1,21 @@
-gem 'thin'
@jferris thoughtbot, inc. member
jferris added a note

Can we just name this Gemfile? The "clean" doesn't add any information for me.

@croaky thoughtbot, inc. member
@jferris thoughtbot, inc. member
jferris added a note

"Gemfile_custom?" "Gemfile_new?" It's hard to come up with a better name. If it can't be just "Gemfile," I'm not too worked up about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jferris jferris commented on the diff
lib/suspenders/app_builder.rb
((8 lines not shown))
def remove_routes_comment_lines
replace_in_file 'config/routes.rb',
/Application\.routes\.draw do.*end/m,
"Application.routes.draw do\nend"
end
- def set_attr_accessibles_on_user
@jferris thoughtbot, inc. member
jferris added a note

Should this be moved into the clearance install generator instead of just removed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jferris jferris commented on the diff
lib/suspenders/app_builder.rb
((20 lines not shown))
def add_email_validator
copy_file 'email_validator.rb', 'app/validators/email_validator.rb'
end
- def include_clearance_matchers
@jferris thoughtbot, inc. member
jferris added a note

This could also move into clearance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jferris jferris commented on an outdated diff
templates/suspenders_layout.html.erb.erb
@@ -9,11 +9,6 @@
</head>
<body class="<%%= body_class %>">
<header>
- <%% if signed_in? -%>
@jferris thoughtbot, inc. member
jferris added a note

I think this is also necessary for the clearance tests to pass, so this would be nice to have in the clearance generator.

@joshuaclayton thoughtbot, inc. member

Without the links, does it make sense to leave the empty <header>?

@croaky thoughtbot, inc. member
croaky added a note

@joshuaclayton Good call. Empty <header> removed in d641ccf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jferris
thoughtbot, inc. member

I think the additions/removals are all good ideas.

I'm a little worried about keeping our Gemfile in sync with new versions of Rails; there are some hidden requirements in the default Gemfile. I'd like to try your approach of just overwriting the Gemfile, but we should keep track of how much crap we have to deal with when new versions of Rails are released.

There were a couple things related to clearance that were useful to have, which I think should be added to the clearance install generator. I'll add an issue to clearance for that.

Good to merge.

@gabebw gabebw commented on the diff
templates/Gemfile_clean
((9 lines not shown))
gem 'airbrake'
+gem 'bourbon'
+gem 'high_voltage'
+gem 'jquery-rails'
+gem 'pg'
gem 'psych'
@gabebw thoughtbot, inc. member
gabebw added a note

Why do we include psych?

@croaky thoughtbot, inc. member
croaky added a note

@gabebw A certain combination of Ruby, Bundler, & Psych causes errors such as:

`<class:Parser>': superclass mismatch for class Mark (TypeError)

bundler/bundler#2068

Including Psych in the bundle fixes the issue.

@mjankowski thoughtbot, inc. member

For that psych error specifically, I always see that error only when the psych gem IS included in the gemfile (because that forces the gem to be installed).

I've found that just removing the Psych gem via gem uninstall psych and allowing the code built into 1.9.3 to be used instead works fine and I don't see that error.

Might be specific to versions of bundler/ruby/rubygems/psych involved?

@gabebw thoughtbot, inc. member
gabebw added a note

@Magnus-G ran into a psych problem (here's a gist from someone else listing the same errors) that prevented him from running rails s. Running gem uninstall psych fixed it.

@croaky thoughtbot, inc. member
croaky added a note

@mjankowski @gabebw @Magnus-G I have seen this error on Heroku with the following stack:

  • Cedar stack
  • Ruby 1.9.3
  • Bundler 1.3.0.pre.5
  • Rails 3.2.11

Including Psych in the Gemfile fixes it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@mike-burns
thoughtbot, inc. member

Exciting changes, looks good to merge.

@croaky croaky closed this
@croaky
thoughtbot, inc. member

Cool. Thanks, Joe and Mike. Squashed into 7c3c377 and merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 17, 2013
  1. @croaky

    Update current gem defaults

    croaky committed
    Provide fewer options and more opinions:
    
    * Add assets group to Gemfile.
    * Add jQuery to Gemfile.
    * Add Postgres to Gemfile.
    * Add Rails to Gemfile.
    * Add Strong Parameters to Gemfile.
    * Alphabetize existing gems.
    * Make Capybara Webkit a default.
    * Remove Clearance.
    * Remove Flutie.
    * Remove Paperclip.
    * Replace Formtastic with Simple Form.
    * Specify minimum version of Capybara Webkit.
    * Specify minimum version of Rails.
Commits on Jan 18, 2013
  1. @croaky

    Remove empty header

    croaky committed
This page is out of date. Refresh to see the latest.
View
20 README.md
@@ -29,10 +29,7 @@ It includes application gems like:
* [Airbrake](/airbrake/airbrake) for exception notification
* [Bourbon](/thoughtbot/bourbon) for Sass mixins
-* [Clearance](/thoughtbot/clearance) for authentication
-* [Flutie](/thoughtbot/flutie) for default CSS styles
-* [Formtastic](/justinfrench/formtastic) for form markup and style
-* [Paperclip](/thoughtbot/paperclip) for file uploads
+* [Simple Form](/plataformatec/simple_form) for form markup and style
And testing gems like:
@@ -79,21 +76,6 @@ This has the same effect as running:
hub create organization/project
-Clearance
----------
-
-You can optionally not include Clearance:
-
- suspenders app --clearance false
-
-Capybara Webkit
----------------
-
-You can optionally not include Capybara Webkit (which depends on QT being
-installed on your machine):
-
- suspenders app --webkit false
-
Dependencies
------------
View
10 features/clearance_false.feature
@@ -1,10 +0,0 @@
-@disable-bundler
-Feature: Skipping Clearance
-
- Scenario: --clearance=false
- When I suspend a project called "test_project" with:
- | argument | value |
- | --clearance | false |
- And I cd to the "test_project" root
- Then "clearance" should not be installed
- And I can cleanly rake the project
View
11 features/webkit_false.feature
@@ -1,11 +0,0 @@
-@disable-bundler
-Feature: Skipping Capybara Webkit
-
- Scenario: --webkit=false
- When I suspend a project called "test_project" with:
- | argument | value |
- | --webkit | false |
- And I cd to the "test_project" root
- Then "capybara-webkit" should not be installed
- And "webkit" should not be included in "spec/spec_helper.rb"
- And I can cleanly rake the project
View
42 lib/suspenders/app_builder.rb
@@ -82,26 +82,14 @@ def create_database
bundle_command 'exec rake db:create'
end
- def set_ruby_to_version_being_used
- inject_into_file 'Gemfile', "\n\nruby '#{RUBY_VERSION}'",
- :after => /source 'https:\/\/rubygems.org'/
- end
-
- def add_custom_gems
- additions_path = find_in_source_paths('Gemfile_additions')
- new_gems = File.open(additions_path).read
- inject_into_file 'Gemfile', "\n#{new_gems}",
- :after => /gem 'jquery-rails'/
+ def replace_gemfile
+ remove_file 'Gemfile'
+ copy_file 'Gemfile_clean', 'Gemfile'
end
- def add_clearance_gem
- inject_into_file 'Gemfile', "\ngem 'clearance'",
- :after => /gem 'jquery-rails'/
- end
-
- def add_capybara_webkit_gem
- inject_into_file 'Gemfile', "\n gem 'capybara-webkit'",
- :after => /gem 'capybara'/
+ def set_ruby_to_version_being_used
+ inject_into_file 'Gemfile', "\n\nruby '#{RUBY_VERSION}'",
+ after: /source 'https:\/\/rubygems.org'/
end
def enable_database_cleaner
@@ -176,10 +164,6 @@ def setup_guard_spork
copy_file 'Guardfile', 'Guardfile'
end
- def generate_clearance
- generate 'clearance:install'
- end
-
def setup_foreman
copy_file 'sample.env', '.sample.env'
copy_file 'Procfile', 'Procfile'
@@ -253,30 +237,16 @@ def customize_error_pages
end
end
- def setup_root_route
- route "root :to => 'Clearance::Sessions#new'"
- end
-
def remove_routes_comment_lines
replace_in_file 'config/routes.rb',
/Application\.routes\.draw do.*end/m,
"Application.routes.draw do\nend"
end
- def set_attr_accessibles_on_user
@jferris thoughtbot, inc. member
jferris added a note

Should this be moved into the clearance install generator instead of just removed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
- inject_into_file 'app/models/user.rb',
- " attr_accessible :email, :password\n",
- :after => /include Clearance::User\n/
- end
-
def add_email_validator
copy_file 'email_validator.rb', 'app/validators/email_validator.rb'
end
- def include_clearance_matchers
@jferris thoughtbot, inc. member
jferris added a note

This could also move into clearance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
- create_file 'spec/support/clearance.rb', "require 'clearance/testing'"
- end
-
def disable_xml_params
copy_file 'disable_xml_params.rb', 'config/initializers/disable_xml_params.rb'
end
View
45 lib/suspenders/generators/app_generator.rb
@@ -3,9 +3,6 @@
module Suspenders
class AppGenerator < Rails::Generators::AppGenerator
- class_option :clearance, :type => :boolean, :aliases => '-C', :default => true,
- :desc => 'Add the Clearance Rails authentication library'
-
class_option :database, :type => :string, :aliases => '-d', :default => 'postgresql',
:desc => "Preconfigure for selected database (options: #{DATABASES.join('/')})"
@@ -18,9 +15,6 @@ class AppGenerator < Rails::Generators::AppGenerator
class_option :skip_test_unit, :type => :boolean, :aliases => '-T', :default => true,
:desc => 'Skip Test::Unit files'
- class_option :webkit, :type => :boolean, :aliases => '-W', :default => true,
- :desc => 'Add the Capybara Webkit Javascript integration testing library'
-
def finish_template
invoke :suspenders_customization
super
@@ -42,7 +36,6 @@ def suspenders_customization
invoke :copy_miscellaneous_files
invoke :customize_error_pages
invoke :remove_routes_comment_lines
- invoke :setup_root_route
invoke :setup_git
invoke :create_heroku_apps
invoke :create_github_repo
@@ -67,11 +60,7 @@ def setup_test_environment
build :generate_rspec
build :configure_rspec
build :enable_database_cleaner
-
- if options[:webkit]
- build :configure_capybara_webkit
- end
-
+ build :configure_capybara_webkit
build :setup_guard_spork
end
@@ -100,17 +89,8 @@ def add_jquery_ui
end
def customize_gemfile
+ build :replace_gemfile
build :set_ruby_to_version_being_used
- build :add_custom_gems
-
- if options[:clearance]
- build :add_clearance_gem
- end
-
- if options[:webkit]
- build :add_capybara_webkit_gem
- end
-
bundle_command 'install --binstubs=bin/stubs'
end
@@ -129,26 +109,12 @@ def configure_app
build :configure_action_mailer
build :configure_time_zone
build :configure_time_formats
-
build :disable_xml_params
-
build :add_email_validator
build :setup_default_rake_task
- build :setup_clearance
build :setup_foreman
end
- def setup_clearance
- if options[:clearance]
- build :generate_clearance
- build :include_clearance_matchers
-
- if using_active_record?
- build :set_attr_accessibles_on_user
- end
- end
- end
-
def setup_stylesheets
say 'Set up stylesheets'
build :setup_stylesheets
@@ -201,13 +167,6 @@ def remove_routes_comment_lines
build :remove_routes_comment_lines
end
- def setup_root_route
- if options[:clearance]
- say 'Setting up a root route'
- build :setup_root_route
- end
- end
-
def outro
say 'Congratulations! You just pulled our suspenders.'
say "Remember to run 'rails generate airbrake' with your API key."
View
32 templates/Gemfile_additions → templates/Gemfile_clean
@@ -1,11 +1,21 @@
-gem 'thin'
@jferris thoughtbot, inc. member
jferris added a note

Can we just name this Gemfile? The "clean" doesn't add any information for me.

@croaky thoughtbot, inc. member
@jferris thoughtbot, inc. member
jferris added a note

"Gemfile_custom?" "Gemfile_new?" It's hard to come up with a better name. If it can't be just "Gemfile," I'm not too worked up about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
-gem 'high_voltage'
-gem 'paperclip'
-gem 'formtastic'
-gem 'flutie'
-gem 'bourbon'
+source 'https://rubygems.org'
+
gem 'airbrake'
+gem 'bourbon'
+gem 'high_voltage'
+gem 'jquery-rails'
+gem 'pg'
gem 'psych'
@gabebw thoughtbot, inc. member
gabebw added a note

Why do we include psych?

@croaky thoughtbot, inc. member
croaky added a note

@gabebw A certain combination of Ruby, Bundler, & Psych causes errors such as:

`<class:Parser>': superclass mismatch for class Mark (TypeError)

bundler/bundler#2068

Including Psych in the bundle fixes the issue.

@mjankowski thoughtbot, inc. member

For that psych error specifically, I always see that error only when the psych gem IS included in the gemfile (because that forces the gem to be installed).

I've found that just removing the Psych gem via gem uninstall psych and allowing the code built into 1.9.3 to be used instead works fine and I don't see that error.

Might be specific to versions of bundler/ruby/rubygems/psych involved?

@gabebw thoughtbot, inc. member
gabebw added a note

@Magnus-G ran into a psych problem (here's a gist from someone else listing the same errors) that prevented him from running rails s. Running gem uninstall psych fixed it.

@croaky thoughtbot, inc. member
croaky added a note

@mjankowski @gabebw @Magnus-G I have seen this error on Heroku with the following stack:

  • Cedar stack
  • Ruby 1.9.3
  • Bundler 1.3.0.pre.5
  • Rails 3.2.11

Including Psych in the Gemfile fixes it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+gem 'rails', '>= 3.2.11'
+gem 'simple_form'
+gem 'strong_parameters'
+gem 'thin'
+
+group :assets do
+ gem 'sass-rails'
+ gem 'coffee-rails'
+ gem 'uglifier'
+end
group :development do
gem 'foreman'
@@ -19,14 +29,14 @@ group :development, :test do
end
group :test do
- gem 'capybara'
- gem 'factory_girl_rails'
- gem 'bourne'
+ gem 'bourne', require: false
+ gem 'capybara-webkit', '>= 0.14.1'
gem 'database_cleaner'
- gem 'timecop'
- gem 'shoulda-matchers'
+ gem 'factory_girl_rails'
gem 'launchy'
+ gem 'shoulda-matchers'
gem 'simplecov', require: false
+ gem 'timecop'
end
group :staging, :production do
View
7 templates/suspenders_layout.html.erb.erb
@@ -8,13 +8,6 @@
<%%= csrf_meta_tags %>
</head>
<body class="<%%= body_class %>">
- <header>
@croaky thoughtbot, inc. member
croaky added a note

@joshuaclayton Good call. Empty <header> removed in d641ccf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
- <%% if signed_in? -%>
- <%%= link_to "Sign out", sign_out_path, :method => :delete %>
- <%% else -%>
- <%%= link_to "Sign in", sign_in_path %>
- <%% end -%>
- </header>
<%%= render 'flashes' -%>
<%%= yield %>
<%%= render 'javascript' %>
Something went wrong with that request. Please try again.