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

Fix first sample in Readme.md #183

Closed
wants to merge 1 commit into from
Closed

Conversation

seriema
Copy link

@seriema seriema commented Nov 24, 2014

Line 26 was res.send(200, {...}) but would give this warning when running:
express deprecated res.send(status, body): Use res.status(status).send(body) instead

Line 32 was .expect('Content-Length', '15') but it gave me an error:
Error: expected "Content-Length" matching 20, got "15"

I've fixed those.

(using Express 4.10.3)

Line 26 was `res.send(200, {...})` but would give this warning when running:
`express deprecated res.send(status, body): Use res.status(status).send(body) instead`

Line 32 was `.expect('Content-Length', '15')` but it gave me an error:
`Error: expected "Content-Length" matching 20, got "15"`

I've fixed those.

(using Express 4.10.3)
@ilanbiala
Copy link

The devDependency is 3.1.0, so not sure if it makes sense to update the example but not the dependency.

@jonathanong
Copy link
Contributor

@seriema if you can update the dev deps as well, that would be great

@seriema
Copy link
Author

seriema commented Feb 9, 2015

Good point @ilanbiala. I thought I was just using a simple test-case.

@jonathanong Changing the devDependency could have a whole lot of consequences I don't feel qualified to test for. =/

My PR's was just to update the Readme so people don't waste as much time as I did when I tried to follow them. I could update this PR with a note on Express versions but I'm not sure what the project would prefer.

@mikelax
Copy link
Contributor

mikelax commented Dec 24, 2015

dep and sample have been updated.

@mikelax mikelax closed this Dec 24, 2015
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