Skip to content

Conversation

@trbngr
Copy link

@trbngr trbngr commented Sep 12, 2015

I need to be able to setHeader

@trbngr
Copy link
Author

trbngr commented Sep 12, 2015

Also opens up logging the proxied request. ;)

@chimurai
Copy link
Owner

Thanks for the PR.

You should already be able to set the request headers with option.headers.
Apart from that; I think it's useful to expose http-proxy's proxyReq event.

Could you document the onProxyReq option in the README and provide a tiny example of its usage?

The code coverage dropped significantly. I think it's caused by describe.only in: https://github.com/chimurai/http-proxy-middleware/pull/28/files#diff-47a0c4f7ac32f4602b09040919af54f2R427

Lastly, could you squash you commits to keep the commits clean?

With these changes, this PR looks fit to be merged. :)

@chimurai
Copy link
Owner

got the chance to update the pull request so it can be merged?

@trbngr
Copy link
Author

trbngr commented Sep 18, 2015

I got so busy that I forgot about it. I'll do it today. ;)

@chimurai
Copy link
Owner

Great! Looking forward to it :)

@chimurai
Copy link
Owner

Any luck to update this PR?

I was hoping to include this on the next release. Otherwise I'll postpone this PR to a later release.

Mean concerns are the dropped code coverage and the documentation of the new option.

@trbngr
Copy link
Author

trbngr commented Sep 26, 2015

My apologies. Life has had me busy and I totally spaced it.

I can get on it right now.

@trbngr
Copy link
Author

trbngr commented Sep 26, 2015

and there she is! 👍

@trbngr
Copy link
Author

trbngr commented Sep 26, 2015

dang it

@trbngr
Copy link
Author

trbngr commented Sep 26, 2015

I should've used a feature branch. Sorry for this.

@trbngr
Copy link
Author

trbngr commented Sep 26, 2015

finally. So sorry.

@trbngr
Copy link
Author

trbngr commented Sep 26, 2015

crap. haha.

@trbngr
Copy link
Author

trbngr commented Sep 26, 2015

Dude. I've made a mess. I went one commit too far. It includes your notes.

@trbngr
Copy link
Author

trbngr commented Sep 26, 2015

If this is a no-go, you can delete this PR and I'll create a new one.

@chimurai
Copy link
Owner

A new PR would be nice, if it isn't too much effort.

@chimurai
Copy link
Owner

Closing PR.
Functionality is implemented in #32

@chimurai chimurai closed this Sep 28, 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.

2 participants