sunlightlabs / opencongress

An open website for opening Congress.
http://www.opencongress.org
GNU General Public License v3.0
48 stars 16 forks source link

rip out metatags from action methods and metaprogram our way into awesom... #488

Closed crdunwel closed 9 years ago

crdunwel commented 9 years ago

...eness

plantfansam commented 9 years ago

This is neat! A couple of things:

Any thoughts, @Cspeisman?

plantfansam commented 9 years ago

Er, that should read "setting the meta tags in the controller action itself"

crdunwel commented 9 years ago

I'm not entirely opposed to putting the metatag methods in the action themselves. I just think it's cluttering the action methods. Also, what if you wanted to get metatags for an action without the accompanying HTML? Separation of concerns is a sound design principle. Another thing to consider is what if you want the metatags to use data generated in the action itself? In the current setup you'd have to put the metatags below the other action code. What if you were rendering a different template than the rails naming convention would otherwise dictate? You'd have metatag code below the other action code but before the render method. That can get pretty messy with metatag code in different places in different actions. In my solution, however, the metatags would always be set right before calling render so all other context variables are assured to be present.

tl;dr

Cspeisman commented 9 years ago

@handlers @crdunwel I'm okay with the solution. Gonna merge.