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 app Rake tasks when -T and --dummy-path is passed to `plugin new` #8139

Merged
merged 1 commit into from Nov 8, 2012

Conversation

Projects
None yet
3 participants
@senny
Member

senny commented Nov 7, 2012

This patch adds the dummy app Rake tasks to the Rakefile template when rails plugin new is called with --skip-test-unit and --dummy-path. Since --dummy-path generates the dummy app even when test unit is skipped I think it's ok to also add in the Rake tasks.

I added a CHANGELOG entry and also created the with_dummy_app? method to reuse the condition.

This is a fix for #8121

@senny

This comment has been minimized.

Show comment
Hide comment
@senny

senny Nov 7, 2012

Member

@drogus can you take a look?

Member

senny commented Nov 7, 2012

@drogus can you take a look?

@drogus

This comment has been minimized.

Show comment
Hide comment
@drogus

drogus Nov 7, 2012

Member

I'm nitpicking ;), but could you format commit message as described here: http://guides.rubyonrails.org/contributing_to_ruby_on_rails.html#commit-your-changes ?

Other than that, it's cool.

Member

drogus commented Nov 7, 2012

I'm nitpicking ;), but could you format commit message as described here: http://guides.rubyonrails.org/contributing_to_ruby_on_rails.html#commit-your-changes ?

Other than that, it's cool.

@senny

This comment has been minimized.

Show comment
Hide comment
@senny

senny Nov 8, 2012

Member

I've read the description in the guide before but I think I forgot because most commit messages in rails don't follow the 50 character limit :) I'll stick to it from now on.

@drogus thanks for the feedback. I've updated the messages.

Member

senny commented Nov 8, 2012

I've read the description in the guide before but I think I forgot because most commit messages in rails don't follow the 50 character limit :) I'll stick to it from now on.

@drogus thanks for the feedback. I've updated the messages.

@carlosantoniodasilva

This comment has been minimized.

Show comment
Hide comment
@carlosantoniodasilva

carlosantoniodasilva Nov 8, 2012

Member

Yeah, the 50 char limit is kinda hard to follow all the time, but I try to at least describe the most I can when I think it's necessary.

@senny this looks good, can you please squash? Thanks!

Member

carlosantoniodasilva commented Nov 8, 2012

Yeah, the 50 char limit is kinda hard to follow all the time, but I try to at least describe the most I can when I think it's necessary.

@senny this looks good, can you please squash? Thanks!

@drogus

This comment has been minimized.

Show comment
Hide comment
@drogus

drogus Nov 8, 2012

Member

@senny yeah, 50 char is tough, but it's not some hard limit, if you go up a little it does not change too much - I just don't like when it's longer than 72, as then it needs to be wrapped on github and starts to be less readable in git log with --oneline

Member

drogus commented Nov 8, 2012

@senny yeah, 50 char is tough, but it's not some hard limit, if you go up a little it does not change too much - I just don't like when it's longer than 72, as then it needs to be wrapped on github and starts to be less readable in git log with --oneline

`plugin new` adds dummy app tasks when necessary.
Closes #8121

The `plugin new` generator always adds the dummy app rake tasks,
when a dummy app was created.
@senny

This comment has been minimized.

Show comment
Hide comment
@senny

senny Nov 8, 2012

Member

@carlosantoniodasilva commits are squashed... (I created separate commits because of the discussion #8043 with @rafaelfranca and I think it makes backporting easier when no random conflicts appear, which are not related.)

Thanks for the inputs regarding commit messages. I think 72 as a hard limit and 50 as a soft limit is reasonable.

Member

senny commented Nov 8, 2012

@carlosantoniodasilva commits are squashed... (I created separate commits because of the discussion #8043 with @rafaelfranca and I think it makes backporting easier when no random conflicts appear, which are not related.)

Thanks for the inputs regarding commit messages. I think 72 as a hard limit and 50 as a soft limit is reasonable.

@carlosantoniodasilva

This comment has been minimized.

Show comment
Hide comment
@carlosantoniodasilva

carlosantoniodasilva Nov 8, 2012

Member

@senny alright, no problem. In this particular case I thought it was not necessary since it was just a minor space in the changelog, which is hardly ever changed - thus conflicts shouldn't be a problem. Thanks!

👍 for the commit messages.

Member

carlosantoniodasilva commented Nov 8, 2012

@senny alright, no problem. In this particular case I thought it was not necessary since it was just a minor space in the changelog, which is hardly ever changed - thus conflicts shouldn't be a problem. Thanks!

👍 for the commit messages.

carlosantoniodasilva added a commit that referenced this pull request Nov 8, 2012

Merge pull request #8139 from senny/8121_engine_generator_rakefile_wi…
…thout_test_unit

Add app Rake tasks when -T and --dummy-path is passed to `plugin new`

@carlosantoniodasilva carlosantoniodasilva merged commit 9abec81 into rails:master Nov 8, 2012

senny added a commit to senny/rails that referenced this pull request Nov 15, 2012

backport #8139, `plugin new` adds dummy app tasks when necessary. …
The `plugin new` generator always adds the dummy app rake tasks,
when a dummy app was created.

Closes #8224

Conflicts:

	railties/CHANGELOG.md

rafaelfranca added a commit that referenced this pull request Nov 15, 2012

Merge pull request #8227 from senny/backport_8139
backport #8139, `plugin new` adds dummy app tasks when necessary
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment