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

Rename makeResponse to response #1672

Merged
merged 1 commit into from May 10, 2018

Conversation

4 participants
@jamiebishop
Copy link
Contributor

jamiebishop commented May 9, 2018

This PR closes #1671, and renames req.makeResponse() to req.response().

Checklist

  • Circle CI is passing (code compiles and passes tests).
  • There are no breaking changes to public API.
  • New test cases have been added where appropriate.
  • All new code has been commented with doc blocks ///.

@tanner0101 tanner0101 changed the base branch from master to 3.1 May 10, 2018

@tanner0101 tanner0101 added this to the 3.1.0 milestone May 10, 2018

@tanner0101 tanner0101 self-assigned this May 10, 2018

@tanner0101
Copy link
Member

tanner0101 left a comment

makeResponse(...) is used a ton in Vapor, and the make prefix as more or less gone away everywhere else in the framework. I think this change makes using this method more convenient and readable. 👍

@tanner0101

This comment has been minimized.

Copy link
Member

tanner0101 commented May 10, 2018

I'm going to merge this through so we can get a 3.1 branch started. If anyone has comments about this change, let's discuss on the 3.1 branch: #1673

@tanner0101 tanner0101 merged commit 741b13c into vapor:3.1 May 10, 2018

3 checks passed

ci/circleci: linux Your tests passed on CircleCI!
Details
ci/circleci: linux-api-template Your tests passed on CircleCI!
Details
ci/circleci: linux-release Your tests passed on CircleCI!
Details
@penny-coin

This comment has been minimized.

Copy link

penny-coin commented May 10, 2018

Hey @jamiebishop, you just merged a pull request, have a coin!

You now have 2 coins.

@tanner0101 tanner0101 referenced this pull request May 10, 2018

Merged

3.1 #1673

1 of 3 tasks complete
@0xTim

This comment has been minimized.

Copy link
Member

0xTim commented May 11, 2018

I’m assuming this would require either a deprecation or be held until Vapor 4 since it contains a breaking change?

@0xTim

This comment has been minimized.

Copy link
Member

0xTim commented May 11, 2018

And in fact I can’t even read, deprecation is in there 🤦‍♂️

@jamiebishop

This comment has been minimized.

Copy link
Contributor Author

jamiebishop commented May 11, 2018

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment