Clarify and correct the description for the --full option of the plugin_... #6499

Merged
merged 1 commit into from Aug 21, 2012

Projects

None yet

6 participants

@lazylester
Contributor

rails plugin new --full generates a full rails application as an engine, but the --help description suggests that the --full option just adds integration testing... it's much more than that

@vijaydev
Member

cc @drogus

@josevalim
Member

That's not exactly what it does. Maybe a better description would be:

"Generate a rails engine with bundled Rails application for testing"

@lazylester
Contributor

hmmm that's not what I observe, Jose...

If I generate a plugin with the --full option, the filesystem includes /app, /app/assets, /app/models, /app/controllers, /app/views, /app/helpers, /app/mailers, i.e. a full Rails application.

without the --full option, these dirs are not created.

in both cases (with/without the --full option) there is another Rails app for integration testing in the /test/dummy directory.

Am I misunderstanding something?

@jackdempsey
Contributor

Yeah this has always been a bit confusing.

IMHO the important difference is:

--full generates a rails engine meant to be included alongside the rest of your application. It's like it operates in the same style, but the files are elsewhere.

--mountable generates a rails engine meant to be mounted at a certain isolated URL.

So --full is like continuing with your app but elsewhere, mountable is more a subcomponent you can embed into a variety of applications.

@josevalim
Member

@lazylester it is not a Rails application because there isn't a config/application.rb file. the bootstrap still happens with an engine defined inside lib/NAME/engine.rb. Maybe a better description would be: "Generates files for app directories, routes, initializers and others" ?

@drogus
Member
drogus commented May 27, 2012

I would also leave the engine there - it's similar to application, but thechnically it's an engine. "Generates files for app directories, routes, initializers and others" seems good to me.

@drogus
Member
drogus commented May 27, 2012

Just the part "the others" bothers me, because it raises a question "what others?".

@josevalim
Member

@drogus generate and you will see :P

@drogus
Member
drogus commented May 27, 2012

@josevalim haha :D right. But you will not be able to tell immediately which files where added by --full. I guess that's fine, because we probably don't want to list everything there, it would be too much of a hassle.

@lazylester
Contributor

@josevalim you're right... I'm being very loose with the word "application", I accept your description. Is there anything I should do to make the committer's job easier? Make another pull request with your suggested wording?

@josevalim and @drogus your work is surely appreciated... thank you!

@carlosantoniodasilva

@lazylester you can just ammend your commit with the new wording, and push force to your branch, that Github updates the pull request. Please do that and ping here, that someone will merge. Thanks!

@lazylester
Contributor

@carlosantoniodasilva I just amended my commit to use @josevalim's suggested wording. Let me know if I need to do anything else. I assume I need to leave this PR open until someone pulls it.

@carlosantoniodasilva

I see you've pushed to your master branch, but this pull request was created from branch patch-1, so you'd have to push force the change to the same branch again to update the pull request, so we can merge using github. If you have any issue, please let us know. Thanks.

@lazylester
Contributor

Sorry to make such a big production out of such a small change! Clearly I'm on the edge of my skill set. Thanks for your patience. I think I got it right this time!

@carlosantoniodasilva

No worries! Now I can see the change correctly, but there are two commits, so you'll need to squash them into one, keeping the commit message like the 1st one:

Clarify and correct the description for the --full option of the plugin_new generator

Thanks!

@lazylester
Contributor

Sorry, I'm confused! I was making the changes/commits/PR's through the github web interface. I don't see a way to squash two commits. I'm not sure how to proceed. Any guidance you can offer would be appreciated.

@carlosantoniodasilva

Ok, no problem. You'd have to clone your repo (with the private/ssh link), and do something like this:

git checkout patch-1
git rebase -i head~2
# an editor should open, you will have to mark the last commit as squash, save and close
git push --force lazylester patch1

After that you should have only 1 commit with the required change. Make sure the message is the clear one, as we commented before.

Take a look here for more info on squashing

Hope that helps, if you have any other doubt, let me know.

@lazylester
Contributor

Thanks for your patience and guidance, Carlos!

@carlosantoniodasilva

You're welcome :). Merging, thanks!

@carlosantoniodasilva carlosantoniodasilva merged commit 11e890f into rails:master Aug 21, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment