search

gojek / heimdall

An enhanced HTTP client for Go
http://gojek.tech
Apache License 2.0
2.63k stars 214 forks source link

Net http error quoting #74

Closed johanavril closed 4 years ago

johanavril commented 4 years ago

fix fail unit test caused by changes in go net/url error message original PR in go

fix typo in the unit test

johanavril commented 4 years ago

tried rebuild the travis CI build, but still failing all unit test already passed

rShetty commented 4 years ago

@johanavril Thanks for the PR. checking this.

coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 87.234% when pulling 13218b440a26ae322606b23fd06d1a9024e049ba on johanavril:net-http-error-quoting into ef69e0bb0cbf72e832303efe6362855811e8cc89 on gojek:master.

rShetty commented 4 years ago

@johanavril Which version of go are you using? I tested it on go 1.13 and tests are working fine.

johanavril commented 4 years ago

@rShetty it works just fine on go 1.13. but the go used in the travis CI use the one from master branch not a released version. I don't know which version of go will implement that changes, but currently the travis CI will fail on the unit test phase.

rShetty commented 4 years ago

@johanavril Earlier it was using the master build of golang, I have changed it to point to the latest stable release go-1.13.6. The tests on the master are working fine now. We should make the above-proposed change once we move to the next stable build. Thoughts?

johanavril commented 4 years ago

@rShetty I agree, that should be the best move.

rShetty commented 4 years ago

@johanavril Thanks for this. Closing this for now.

johanavril commented 4 years ago

hi @rShetty , go 1.14 has been released, and the changes on the error quoting is also on the 1.14 version. I think it would be better to update go version on .travis.yml and re-open this PR, any thoughts ?