Skip to content

fix http route recursion#228

Merged
3rd-Eden merged 2 commits intoflatiron:masterfrom
shadedyin:http-recursion
Jan 6, 2015
Merged

fix http route recursion#228
3rd-Eden merged 2 commits intoflatiron:masterfrom
shadedyin:http-recursion

Conversation

@shadedyin
Copy link
Copy Markdown
Contributor

Not much to it. Oh, and I wrote you a unit test to add to your collection.

@shadedyin
Copy link
Copy Markdown
Contributor Author

I suspected my changes may have had unexpected consequences. Fortunately the breakage seems minor and specific.

@shadedyin shadedyin closed this Apr 1, 2014
jhoff pushed a commit to jhoff/director that referenced this pull request Jun 14, 2014
@beaugunderson beaugunderson reopened this Jun 25, 2014
@beaugunderson
Copy link
Copy Markdown
Contributor

@psev Any reason you closed this? I'd like to merge it.

@cookpete
Copy link
Copy Markdown

Any update on this? Any reason it hasn't been merged?

3rd-Eden added a commit that referenced this pull request Jan 6, 2015
@3rd-Eden 3rd-Eden merged commit f07509f into flatiron:master Jan 6, 2015
@3rd-Eden
Copy link
Copy Markdown
Member

3rd-Eden commented Jan 6, 2015

Thanks for your contribution. <3

@shadedyin
Copy link
Copy Markdown
Contributor Author

Wow, I can't remember what I was thinking when I closed this, my apologies.

I thoroughly went through Director's source and really liked what I saw. I found a problem with recursively calling the path function so I set about fixing a few minor issues, which eventually turned into a complete rewrite. You can see that Elixir.js is entirely based on the Director git repository.

The utility functions at the beginning of router.js have been factored and reworked into RaptorJuice.js, which also includes lightweight generators/iterators. Director turned into Elixir.js, which builds solely upon RaptorJuice.

Both projects are very thoroughly documented, tested and above 98% code coverage. Only a few lines need to be changed for javascript isomorphism. I'll get back to it eventually, I've just been way busy with a new DevOps job.

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