Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

reimplement as subclass of ActionDispatch::Static #3

Merged
merged 1 commit into from Nov 29, 2013

Conversation

Projects
None yet
2 participants
Contributor

jjb commented Nov 29, 2013

  • there was a lot of copy-pasted code from ActionDispatch::Static
  • now the code is smaller and smaller is good
  • if other behavior or api changes with ActionDispatch::Static, this class will automatically get it
  • an actual clear use case for inheritance, yay!
  • as such, maybe easier for ActionDispatch::Static maintainers to grok and to consider for integration of features upstream

minor todo/problem: tighter lockdown of version dependencies for actionpack? (both min and max versions).

What do you think?

Owner

jbaudanza commented Nov 29, 2013

Looks good to me. I assume all the tests pass?

Contributor

jjb commented Nov 29, 2013

yep. although i haven't actually tried the gem. doing so now...

On Fri, Nov 29, 2013 at 11:54 AM, Jonathan Baudanza <
notifications@github.com> wrote:

Looks good to me. I assume all the tests pass?


Reply to this email directly or view it on GitHubhttps://github.com/jbaudanza/action_dispatch-gz_static/pull/3#issuecomment-29527366
.

Contributor

jjb commented Nov 29, 2013

okay, i did a few checks with a local app, all looks good!

@jbaudanza jbaudanza added a commit that referenced this pull request Nov 29, 2013

@jbaudanza jbaudanza Merge pull request #3 from jjb/inherits-from-actiondispatch-static
reimplement as subclass of ActionDispatch::Static
f2d24a9

@jbaudanza jbaudanza merged commit f2d24a9 into jbaudanza:master Nov 29, 2013

Contributor

jjb commented Nov 29, 2013

cool!

@jjb jjb deleted the jjb:inherits-from-actiondispatch-static branch Nov 29, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment