search

rgrove / combohandler

A simple Yahoo!-style combo handler in Node.js.
MIT License
116 stars 32 forks source link

Add capability to handle dynamic placeholders in route config #6

Closed evocateur closed 11 years ago

evocateur commented 12 years ago

The functionality I told you about at YUIConf, cleaned up a bit. It is pretty tightly bound to the instance closure, but I couldn't figure out a good/safe/effective way to make it into a middleware component. This functionality is live on Zillow.com

rgrove commented 12 years ago

Sweet! Thanks for this and the other pull requests. At a glance, all this functionality looks awesome and I want to bring it in, but I'd also like to try to move it into middleware if possible. I guess I should also finally write some unit tests to keep myself from breaking stuff.

I won't have time to work on this until next week at the earliest, but look for some comments/commits from me over the holidays.

evocateur commented 12 years ago

No problem, I understand. Once you've picked a test framework/pattern, let me know and I'll try to find time to contribute to those as well. The versioned-roots thing (currently live) could definitely benefit!

rgrove commented 12 years ago

I decided to go with Mocha for tests. It's working well so far.

I've added lots of tests and merged two of your pull requests. Need to figure out a middleware strategy before I can merge the remaining two.

evocateur commented 12 years ago

Great!

My ops team is pester^H^H^Hasking me to figure out some monitor flapping (service appearing unresponsive and taken out of load-balancer pool) issues in production, so I'll have plenty of opportunity next week to write some more unit tests. (Although I suspect the issue lies more at the LearnBoost/cluster level, which may end up with me forking and trying to make work with node 0.6.x...)

rgrove commented 12 years ago

Let me know if you do end up getting cluster to run on 0.6.x. I want that bad.

evocateur commented 11 years ago

This pull request is tremendously out of date, and I've been noodling about the middleware strategy enough that my implementation will be drastically different. I'll open another PR when I'm ready.

(Oh, and I do have the new cluster working in my fork's zcombo branch. Just gotta bring it up to date with 0.2.1)