Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implements #8 content-range using code from streamer.js #93

Closed
wants to merge 3 commits into from

Conversation

choonkeat
Copy link
Contributor

No description provided.

@jfhbrook
Copy link
Owner

No way! Awesome.

I've been really lame at maintaining recently, but I'll try to check this puppy out today. This has been a long time coming!

@jfhbrook
Copy link
Owner

Hey, so this fails a number of tests for me. Can you make it so that npm test passes? Then I'll take another look. :)

@choonkeat
Copy link
Contributor Author

Sorry about that. It worked for my purpose and I pushed it out too eagerly. Updated with fix

@jfhbrook
Copy link
Owner

Okay cool, tests pass now.

Looks sane enough. I don't see any new test coverage, though. Can you add a basic test case? The malformed tests, for example, just spin up a server and make some requests/asserts to make sure it's working.

@max-mapper
Copy link

ooooh I want this!

@jfhbrook
Copy link
Owner

jfhbrook commented Feb 9, 2014

@maxogden how would you feel about adding a sanity test?

@jfhbrook
Copy link
Owner

jfhbrook commented Feb 9, 2014

tbh this looks annoying to test, so I'm gonna merge without.

@jfhbrook
Copy link
Owner

jfhbrook commented Feb 9, 2014

v0.5.0 should be published. Let me know if anything.

@jfhbrook jfhbrook closed this Feb 9, 2014
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.

None yet

3 participants