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

Use the new RequestsWrapper for connecting to services #4094

Merged
merged 3 commits into from
Jul 11, 2019
Merged

Use the new RequestsWrapper for connecting to services #4094

merged 3 commits into from
Jul 11, 2019

Conversation

ofek
Copy link
Contributor

@ofek ofek commented Jul 11, 2019

Motivation

Standardize configuration, esp. for proxy behavior

@codecov
Copy link

codecov bot commented Jul 11, 2019

Codecov Report

Merging #4094 into master will decrease coverage by 4.74%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4094      +/-   ##
==========================================
- Coverage   86.55%   81.81%   -4.75%     
==========================================
  Files         768        7     -761     
  Lines       41365      352   -41013     
  Branches     5009       76    -4933     
==========================================
- Hits        35805      288   -35517     
+ Misses       4193       27    -4166     
+ Partials     1367       37    -1330

Copy link
Member

@AlexandreYang AlexandreYang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a fix for #4094 (comment)

Otherwise 👍

ofek and others added 2 commits July 11, 2019 10:11
@ofek ofek merged commit 7c7b6a2 into master Jul 11, 2019
@ofek ofek deleted the ofek/mr branch July 11, 2019 16:07
@AlexandreYang
Copy link
Member

AlexandreYang commented Jul 18, 2019

@ofek Was # user: <USERNAME> a typo that you fixed ? seems the user config is not used.

cc @l0k0ms

@l0k0ms
Copy link
Contributor

l0k0ms commented Jul 18, 2019

@AlexandreYang should we fix it again?

@ofek
Copy link
Contributor Author

ofek commented Jul 18, 2019

@AlexandreYang Yes unused

@AlexandreYang
Copy link
Member

AlexandreYang commented Jul 19, 2019

Looks ok then. Thx for the confirmation.

@AlexandreYang should we fix it again?

@l0k0ms nothing more to fix, this PR is actually the fix :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants