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

Introduce new preRequest event and emit request on request event #235

Merged
merged 5 commits into from Mar 5, 2019

Conversation

@tlivings
Copy link
Contributor

tlivings commented Feb 8, 2019

Closes #209

tlivings added 3 commits Nov 20, 2018
…nt now emits created request. #209

updated README
@geek

This comment has been minimized.

Copy link
Member

geek commented Feb 8, 2019

@tlivings thoughts on keeping request how it was and we can say it's deprecated in the readme. Then where you have request now (after the request) we can rename to onPostRequest. Also, should we name it onPreRequest?

@tlivings

This comment has been minimized.

Copy link
Contributor Author

tlivings commented Feb 9, 2019

on(‘onPreRequest’ ...) 🤔 seems redundant but sure.

My only problem with onPostRequest is that it sounds like after a request is made.

@geek

This comment has been minimized.

Copy link
Member

geek commented Feb 9, 2019

Ya, I don't know what I was thinking. The name you have is good.

@geek

This comment has been minimized.

Copy link
Member

geek commented Feb 9, 2019

@tlivings I'm going to merge as an enhancement... I'll move the existing request alongside preRequest so that it's not a breaking change. Then mark it as deprecated in the readme and recommend people to use the preRequest event. How does that sound?

@geek geek added the feature label Feb 9, 2019
@geek geek added this to the 14.1.5 milestone Feb 9, 2019
lib/index.js Outdated Show resolved Hide resolved
@geek geek removed this from the 14.2.0 milestone Mar 5, 2019
geek added 2 commits Mar 5, 2019
@geek
geek approved these changes Mar 5, 2019
@geek geek added this to the 15.0.0 milestone Mar 5, 2019
@geek geek self-assigned this Mar 5, 2019
@geek geek merged commit ea0cc5f into hapijs:master Mar 5, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@geek geek mentioned this pull request Apr 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.