Skip to content

generator to create representer with decorator pattern#84

Open
amkirwan wants to merge 1 commit intotrailblazer:masterfrom
amkirwan:templates
Open

generator to create representer with decorator pattern#84
amkirwan wants to merge 1 commit intotrailblazer:masterfrom
amkirwan:templates

Conversation

@amkirwan
Copy link
Copy Markdown

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
Copy Markdown

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

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

@ianks
Copy link
Copy Markdown

ianks commented Oct 3, 2014

I prefer decorator as default as well.

@summera
Copy link
Copy Markdown

summera commented Oct 3, 2014

👍 for decorator as default

@apotonick
Copy link
Copy Markdown
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