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 option to model generator to generate classes with ActiveRecord::Model #4174

Conversation

guilleiguaran
Copy link
Member

This add an option to model generator to generate classes that use ActiveRecord::Model. Ex:

rails g model Post --with-include

will generate:

class Post
  include ActiveRecord::Model
end

Please don't merge this inmediatly, I think we can choose a better option name than --with-include for this

…d::Model include instead of inherit from ActiveRecord::Base
@guilleiguaran
Copy link
Member Author

cc @jonleighton

@jonleighton
Copy link
Member

Thanks for the patch.

However, I'm not sure about this:

  • Feels premature, I'd rather wait and see what the take-up on AR::Model is before adding extra 'stuff' for it
  • Not a huge gain - is not much to type out manually
  • Alternatively, we might consider adding an app-wide option in the future instead
  • Alternatively, we might just switch to generating with AR::Model by default in the future, when it is more proven/stable

But, I am biased because I never use generators (except for migrations).

cc @josevalim @tenderlove

Merry christmas! ho ho ho etc :)

@tenderlove
Copy link
Member

TBH, I'd like to see this bake in master for a while before we start adding generators. Right now, I think this is an "advanced users" feature, and I don't think advanced users need / user generators. But that's my 2cents.

@guilleiguaran
Copy link
Member Author

Agree, we can leave this open and revisit it on the next months

@isaacsanders
Copy link
Contributor

I 👍 this. Any thoughts on this @tenderlove / @wycats?

@josevalim
Copy link
Contributor

I still think the points given by @jonleighton and @tenderlove still apply. We will only be able to decide this after 4.0 is out.

@carlosantoniodasilva
Copy link
Member

Hey guys, do we keep this around opened for more time, or should we consider it closed?

@rafaelfranca
Copy link
Member

Lets just close it.

Closed.

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

7 participants