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

Move tunnel logic into separate module #1631

Merged
merged 3 commits into from Jun 15, 2015
Merged

Conversation

simov
Copy link
Member

@simov simov commented Jun 6, 2015

Following the current convention for the rest of the internal modules in request I moved out the tunnel logic. The first two commits are strictly non breaking changes. In the third one I removed 3 properties out of the request instance.

I'm overriding the proxyHeaderWhiteList and proxyHeaderExclusiveList via the options argument in setup. In future I would like to not rely on extending the instance in the request ctor, so this approach will help me on the next step of the refactoring. In future I would like to have a strict mechanism with corresponding tests to explicitly define all of the expected public properties for the request instance. This will give us the flexibility to make internal changes more easily, as right now it's not clear if a property is set on the request instance with the intention to be public, or just to carry state through out the instance. And of course someone down the line might actually rely on it.

Also what I noticed is that options.proxyHeaderExclusiveList extends defaultProxyHeaderExclusiveList, but options.proxyHeaderWhiteList overrides defaultProxyHeaderWhiteList. Changing options.proxyHeaderWhiteList to extend defaultProxyHeaderWhiteList doesn't break anything, therefore we don't have tests for that behavior, so I left it as it is.

//cc @mikeal @nylen

simov added 3 commits June 6, 2015 10:50
- proxyHeaderWhiteList, proxyHeaderExclusiveList and proxyHeaders
  are no longer public properties of the request instance
simov added a commit that referenced this pull request Jun 15, 2015
Move tunnel logic into separate module
@simov simov merged commit 7d0ee92 into request:master Jun 15, 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.

None yet

1 participant