Skip to content

Conversation

@wonderix
Copy link
Contributor

@wonderix wonderix commented Jun 1, 2020

My original PR is working when the headers are passed in the constructor to newHttpClient

client = newHttpClient(headers=newHttpHeader(...,titleCase=true))

but it is not working in case the headers are passed in each request.

client.request(url,HttpGet,headers=newHttpHeader(...,titleCase=true))

This is caused by the fact that a new HttpHeaderobject is created in override.

Therefore, I would like to revert my original PR and propose a new solution.

@Araq Araq requested a review from dom96 June 1, 2020 17:22
@dom96
Copy link
Contributor

dom96 commented Jun 1, 2020

Did your last implementation make it into a release? If so we will be breaking an API that was in a released version... I guess we can be pragmatic here, but ugh. Any chance you could fix the problem instead of reimplementing your original solution?

@wonderix
Copy link
Contributor Author

wonderix commented Jun 1, 2020

The last PR was merged after Release 1.2.0. Therefore, I suggested a new implementation. It would also be possible to fix the problem. But I'm not sure how to merge two HttpHeaders with different titleCase setting in override

@wonderix
Copy link
Contributor Author

wonderix commented Jun 2, 2020

A solution without breaking change might look like this

@Araq
Copy link
Member

Araq commented Jun 2, 2020

Since it happened after 1.2, let's break it.

@dom96
Copy link
Contributor

dom96 commented Jun 12, 2020

Sure, let's keep breaking things, even when the alternative is much better. Seriously, look at @wonderix's link, that's what you should PR instead. It makes sense to have this option as part of the HttpHeaders anyway.

@dom96 dom96 closed this Jun 12, 2020
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.

3 participants