prismicio / ruby-rails-starter

Starter project for Ruby on Rails – Works with any prismic.io repository
32 stars 21 forks source link

Factorisation of prismic-specific controller methods #12

Closed rudyrigot closed 10 years ago

rudyrigot commented 10 years ago

I just put everything in a new PrismicController (what was in PrismicOauthController and the common part that was always in ApplicationController), and now, if you wish your ApplicationController actions to be able to use prismic.io, you simply need to have it extend PrismicController.

@dohzya, does it sound good to you?

If so, I'll generalize it to other examples we have, and then I'll make a generator for it and for the PrismicService and all, in the gem, if you agree it's a good idea.

dohzya commented 10 years ago

I see one problem: because this solution implies to extends PrismicController, it is not possible for an user to have a global controller whose evert other controllers extend.

I think we could move all methods into a PrismicController module instead, which can be extended by every controller, whatever their parent is. This way we preserve the separation between the standard helpers and the OAuth-specific actions (are we don't pollute every controllers with them).

What do you think?

rudyrigot commented 10 years ago

I think it makes a lot of sense; I'll look at it today.

rudyrigot commented 10 years ago

Alright, it's updated, and I also updated the doc. Does it look good to you?

dohzya commented 10 years ago

It still injects oauth-specific methods in every controllers including PrismicController. But I don't think it is a big problem, so it can be merged as this.

I let you make the split or merge directly, as you feel ;-)

:+1:

rudyrigot commented 10 years ago

Aaaaah I hadn't understood that part of the idea; ok, I've updated it, can you have another look the make sure I got you right? I'll start looking at Rails generators meanwhile.

rudyrigot commented 10 years ago

I'll merge this, because my latest change (the only one that wasn't peer-reviewed) is small, and this PR contains a far better README documentation (and I'm actually revamping documentations, on the website, and in GitHub).

rudyrigot commented 10 years ago

Alright, all the example follow that convention now too, it's all up-to-date.