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

generator to create representer with decorator pattern #84

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

amkirwan
Copy link

Added an option to the generator to create representers using the decorator pattern

bin/rails g representer -d Singer or bin/rails g representer --decorator Singer generates:

class Singer < Roar::Decorator
end

@ianks
Copy link

ianks commented Sep 30, 2014

cool stuff! Unfortunately this will have some serious merge conflicts with the scaffold_controller feature I am pull requesting (#83).

If @apotonick approves of this, we could implement this feature branch I am working on. That would save some headaches and get this feature in quicker IMO!

@apotonick
Copy link
Member

Cool idea, @amkirwan. Also, what if we make Decorator default?

@ianks I'll check out the scaffold PR, then we can coordinate integrating @amkirwan's work.

@amkirwan
Copy link
Author

amkirwan commented Oct 1, 2014

@apotonick I can make the Decorator the default if you think that makes the most sense going forward for the project.

@apotonick
Copy link
Member

Well, what do you think? I just like decorator better, it's a tiny little bit faster and does not pollute the model. On the other hand, I guess most people use modules, so maybe keep it.

@amkirwan
Copy link
Author

amkirwan commented Oct 3, 2014

Personally I like the decorator a lot better for all the reasons that you mentioned. I'd say switch it to the default.

@apotonick
Copy link
Member

Hmmm.... it's really up to us. Maybe you're right.

@ianks
Copy link

ianks commented Oct 3, 2014

I prefer decorator as default as well.

@summera
Copy link

summera commented Oct 3, 2014

👍 for decorator as default

@apotonick
Copy link
Member

Think we got a winner. 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants