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

Add twurl version to user agent header #126

Merged
merged 2 commits into from
Dec 8, 2019

Conversation

smaeda-ks
Copy link
Collaborator

@smaeda-ks smaeda-ks commented Dec 4, 2019

Historically, twurl always uses User-Agent: (OAuth gem v0.x.x) request header that comes from the oauth gem. For a better usage tracking/analytics purpose, we should consider adding a client's twurl, Ruby, and platform versions.

Original PR was opened in #50 but since it's too old, I'm submitting a new PR for this. Also, with the original PR, it still remains (OAuth gem v0.x.x) strings because of the oauth gem adds it on-the-fly. So I'm modifying it right after the request.oauth!() call rather than adding it in the request controller.

Example:
User-Agent: twurl version: 0.9.3 platform: ruby 2.6.1 (x86_64-darwin17)

@smaeda-ks smaeda-ks self-assigned this Dec 4, 2019
@smaeda-ks smaeda-ks requested a review from a team December 4, 2019 18:48
@smaeda-ks smaeda-ks added this to the Target v0.9.4 milestone Dec 4, 2019
@coveralls
Copy link

coveralls commented Dec 4, 2019

Coverage Status

Coverage increased (+0.08%) to 88.235% when pulling 5e2a23c on smaeda-ks:smaeda-ks/custom-user-agent into 65632de on twitter:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 88.13% when pulling 65ce8fb on smaeda-ks:smaeda-ks/custom-user-agent into 65632de on twitter:master.

@smaeda-ks
Copy link
Collaborator Author

cc: @andypiper This one is pretty straightforward I believe. Thanks!

@andypiper
Copy link
Contributor

LGTM

@smaeda-ks
Copy link
Collaborator Author

@andypiper Thank you!

@smaeda-ks smaeda-ks merged commit d037cf6 into twitter:master Dec 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants