-
-
Notifications
You must be signed in to change notification settings - Fork 16.7k
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
Remove res.jsonp(status, obj) signature #2940
Remove res.jsonp(status, obj) signature #2940
Conversation
@@ -25,6 +25,7 @@ This is the first Express 5.0 alpha release, based off 4.10.1. | |||
- `req.acceptsLanguage` - use `req.acceptsLanguages` | |||
- `res.json(obj, status)` signature - use `res.json(status, obj)` | |||
- `res.jsonp(obj, status)` signature - use `res.jsonp(status, obj)` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this existing line be rewritten to:
- `res.jsonp(obj, status)` signature - use `res.status(status).jsonp(obj)`
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, because that is the change log for the first alpha. You can't changed what has already been released.
03eb2da
to
37c8352
Compare
Fixed the History.md mistake. |
37c8352
to
05c9a5a
Compare
Updated due to comments on similar pull request (#2939) |
@tunniclm coverage has decreased by a fraction, otherwise looks good to me. |
As mentioned in the other similar pull request (see #2939 (comment)), the slight reduction in coverage is due to the reduced number of lines in the changed file (response.js). |
05c9a5a
to
2f2c636
Compare
Merged into the 5.0 branch :) |
No description provided.