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

Fixed a bug causing duplicate user-agent headers when using a shared HttpClient. #2008

Merged
merged 3 commits into from
Mar 12, 2023

Conversation

tacosontitan
Copy link
Contributor

Description

This PR fixes issue #1990 by using string interpolation to concatenate the Product and Version properties in a way that matches the UserAgent property of RestClientOptions.

Purpose

This pull request is a:

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Sorry, something went wrong.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…ient.
@dnfadmin
Copy link

dnfadmin commented Feb 20, 2023

CLA assistant check
All CLA requirements met.

@tacosontitan
Copy link
Contributor Author

I'm not seeing why the test is exiting with code 1. I'm more than willing to make adjustments if needed. Can someone help me identify what's going wrong there?

@tacosontitan
Copy link
Contributor Author

Nevermind, I found it. Failed 1 test, it was hidden away at first. I'll review and update once resolved.

@tacosontitan
Copy link
Contributor Author

After local review, the test in question fails in the dev branch without my change. I tried reviewing the action history for confirmation, but that yielded no results so I must leave it to the project maintainers at this point.

@alexeyzimarev
Copy link
Member

Deprecated by #2010

@tacosontitan
Copy link
Contributor Author

@alexeyzimarev, I reviewed the files changed in #2010 and don't see where this was accounted for. Was the correct item linked?

@alexeyzimarev
Copy link
Member

Ah sorry. I might have messed up. Let me check it now. Apologies!

@alexeyzimarev alexeyzimarev reopened this Mar 12, 2023
@alexeyzimarev
Copy link
Member

@tacosontitan Cloud you check the message from the CLA bot? It's a new thing from .NET Foundation, would be nice if you can do what it asks.

@tacosontitan
Copy link
Contributor Author

@dotnet-policy-service agree

@tacosontitan
Copy link
Contributor Author

@alexeyzimarev, no problem, also had to update the PR with the latest changes in dev.

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

3 participants