Conversation
|
Instead of adding another function, I'd rather see this become part of the settings. I'd imagine it would be a |
|
I can see that there might be some other proxy settings which we want to override per connection, As for Now if we can break clients, we can have one function doing all that i.e. we drop |
|
I'm trying to parse the comments and the PR, but my brain's having trouble keeping up (it might be the cold). Firstly, the naming doesn't make sense to me ( I'm also not certain what what you mean by "breaking clients." Do you mean users of the API? I think the purpose of the new function has something to do with efficiency and not needing to dispatch on the request value twice. But it seems like that could be handled by simply including a |
|
The idea was to be able to modify proxy settings based on the request. |
|
I understood that goal, and I thought my recommendation addressed that:
I don't understand the content of the following commit that was added, which seemed to do something very different. |
|
OK, so let me understand it: the new use should be: or something safer then |
|
I'd personally use something safer than |
|
Hi, just wonder which tests can I run to confirm that those changes does not break older code, nor lock resources. |
|
Actually, I just wasn't aware that you'd updated the PR. Is your question a polite way of letting me know you think this is ready to be merged? |
|
Not really, I want to test it better. It improves my use cases. |
|
For breaking older code, there's nothing automated, just code review. I'm not sure which resources you're worried about locking, can you clarify? |
|
The idea was to move So I wanted to see if this change is still OK with snoyberg/keter#29 |
|
I think it's still OK |
d23e829 to
3e85094
Compare
|
I squashed commits, and used |
|
I merged this to a separate branch and added one more commit on top: Would that work for you? |
This commit adds:
waiProxyToSettingsWithTimeoutsWhich is a copy of the
waiProxyToSettingsbut it get timeout bounds from thegetDestcall, and usestimeoutfromketer.The idea is that we might need to have different time-outs depending on say destination.
Apply
hlintandstylish-haskellsuggestions.