Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an after_bundle callback in Rails templates #16359

Merged
merged 2 commits into from Aug 6, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions guides/source/4_2_release_notes.md
Expand Up @@ -85,6 +85,9 @@ Please refer to the [Changelog][railties] for detailed changes.
* Introduced `Rails.gem_version` as a convenience method to return `Gem::Version.new(Rails.version)`.
([Pull Request](https://github.com/rails/rails/pull/14101))

* Introduced an `after_bundle` callback in the Rails templates.
([Pull Request](https://github.com/rails/rails/pull/16359))


Action Pack
-----------
Expand Down
24 changes: 21 additions & 3 deletions guides/source/rails_application_templates.md
Expand Up @@ -38,9 +38,11 @@ generate(:scaffold, "person name:string")
route "root to: 'people#index'"
rake("db:migrate")

git :init
git add: "."
git commit: %Q{ -m 'Initial commit' }
after_bundle do
git :init
git add: "."
git commit: %Q{ -m 'Initial commit' }
end
```

The following sections outline the primary methods provided by the API:
Expand Down Expand Up @@ -228,6 +230,22 @@ git add: "."
git commit: "-a -m 'Initial commit'"
```

### after_bundle(&block)

Registers a callback to be executed after bundler and binstub generation have
run. Useful for adding the binstubs to version control:

```ruby
after_bundle do
git :init
git add: '.'
git commit: "-a -m 'Initial commit'"
end
```

The callbacks gets executed even if `--skip-bundle` and/or `--skip-spring` has
been passed.

Advanced Usage
--------------

Expand Down
32 changes: 32 additions & 0 deletions guides/source/upgrading_ruby_on_rails.md
Expand Up @@ -58,6 +58,38 @@ When assigning `nil` to a serialized attribute, it will be saved to the database
as `NULL` instead of passing the `nil` value through the coder (e.g. `"null"`
when using the `JSON` coder).

### `after_bundle` in Rails templates

If you have a Rails template that adds all the files in version control, it
fails to add the generated binstubs because it gets executed before Bundler:

```ruby
# template.rb
generate(:scaffold, "person name:string")
route "root to: 'people#index'"
rake("db:migrate")

git :init
git add: "."
git commit: %Q{ -m 'Initial commit' }
```

You can now wrap the `git` calls in an `after_bundle` block. It will be run
after the binstubs have been generated.

```ruby
# template.rb
generate(:scaffold, "person name:string")
route "root to: 'people#index'"
rake("db:migrate")

after_bundle do
git :init
git add: "."
git commit: %Q{ -m 'Initial commit' }
end
```

Upgrading from Rails 4.0 to Rails 4.1
-------------------------------------

Expand Down
7 changes: 7 additions & 0 deletions railties/CHANGELOG.md
@@ -1,3 +1,10 @@
* Add `after_bundle` callbacks in Rails templates. Useful for allowing the
generated binstubs to be added to version control.

Fixes #16292.

*Stefan Kanev*

* Scaffold generator `_form` partial adds `class="field"` for password
confirmation fields.

Expand Down
11 changes: 11 additions & 0 deletions railties/lib/rails/generators/actions.rb
Expand Up @@ -7,6 +7,7 @@ module Actions
def initialize(*) # :nodoc:
super
@in_group = nil
@after_bundle_callbacks = []
end

# Adds an entry into +Gemfile+ for the supplied gem.
Expand Down Expand Up @@ -232,6 +233,16 @@ def readme(path)
log File.read(find_in_source_paths(path))
end

# Registers a callback to be executed after bundle and spring binstubs
# have run.
#
# after_bundle do
# git add: '.'
# end
def after_bundle(&block)
@after_bundle_callbacks << block
end

protected

# Define log for backwards compatibility. If just one argument is sent,
Expand Down
6 changes: 6 additions & 0 deletions railties/lib/rails/generators/rails/app/app_generator.rb
Expand Up @@ -259,6 +259,12 @@ def finish_template
public_task :apply_rails_template, :run_bundle
public_task :generate_spring_binstubs

def run_after_bundle_callbacks
@after_bundle_callbacks.each do |callback|
callback.call
end
end

protected

def self.banner
Expand Down
15 changes: 15 additions & 0 deletions railties/test/generators/app_generator_test.rb
Expand Up @@ -501,6 +501,21 @@ def test_psych_gem
end
end

def test_after_bundle_callback
path = 'http://example.org/rails_template'
template = %{ after_bundle { run 'echo ran after_bundle' } }
template.instance_eval "def read; self; end" # Make the string respond to read

generator([destination_root], template: path).expects(:open).with(path, 'Accept' => 'application/x-thor-template').returns(template)
Copy link
Member

Choose a reason for hiding this comment

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

Could you avoid using Mocha please ? We are trying to remove it from the test suite. Thanks for the patch so far, it looks great! 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can try, but I'm not sure how. I arrived at this code by following the style of the surrounding tests. I want to check that:

  1. `bundle gets run
  2. ...then binstubs get generated
  3. ...then the after_bundle callbacks get executed

Surely the test cannot invoke bundle, since it will make them (1) nondeterministic and (2) dependant on internet connection. Also, (3) very slow. Calls to the first two need to get stubbed in some way.

If I'm not allowed to use Mocha, I'm not sure how to handle it. I can define singleton methods on generator, but that will surely get messier and harder to understand.

There is also the business of supplying a template. Since a string containing code cannot be passed to generator, there's this path and template.instance_eval business that I borrowed from a test in the same suite. If I'm to remove Mocha, we should also figure out how to do that.

Finally, if the other tests in the suite can benefit from the same approach. Maybe there are a few nice abstractions that can be extracted and all the tests can benefit from them. This feels like another, larger task. I'd love to do it also (in a separate PR possibly), but I'll definitely need some hints on how to do it :)

Copy link
Member

Choose a reason for hiding this comment

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

@robin850 given that the surrounding tests use the same pattern I don't think we need to tackle this in the same PR. Basically if one does remove Mocha for one of the tests it should be trivial to remove it for the others. They are all doing mostly the same stuff.

Copy link
Member

Choose a reason for hiding this comment

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

Yes. No bother with mocha. I'll deal with it.

Copy link
Member

Choose a reason for hiding this comment

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

given that the surrounding tests use the same pattern I don't think we need to tackle this in the same PR

Ah fair point 👍


bundler_first = sequence('bundle, binstubs, after_bundle')
generator.expects(:bundle_command).with('install').once.in_sequence(bundler_first)
generator.expects(:bundle_command).with('exec spring binstub --all').in_sequence(bundler_first)
generator.expects(:run).with('echo ran after_bundle').in_sequence(bundler_first)

quietly { generator.invoke_all }
end

protected

def action(*args, &block)
Expand Down