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

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

Merged
merged 1 commit into from
Aug 21, 2012
Merged

Conversation

lazylester
Copy link
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
Copy link
Member

cc @drogus

@josevalim
Copy link
Contributor

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

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

@lazylester
Copy link
Contributor Author

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
Copy link
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
Copy link
Contributor

@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
Copy link
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
Copy link
Member

drogus commented May 27, 2012

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

@josevalim
Copy link
Contributor

@drogus generate and you will see :P

@drogus
Copy link
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
Copy link
Contributor Author

@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
Copy link
Member

@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
Copy link
Contributor Author

@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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

Thanks for your patience and guidance, Carlos!

@carlosantoniodasilva
Copy link
Member

You're welcome :). Merging, thanks!

carlosantoniodasilva added a commit that referenced this pull request Aug 21, 2012
Clarify and correct the description for the --full option of the 
plugin_new generator. [ci skip]
@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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants