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.

Sign up for GitHub

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

Jump to bottom

Add node/version to the user agent header. #299

Merged
merged 2 commits into from
Sep 13, 2022
Merged

Add node/version to the user agent header. #299

merged 2 commits into from
Sep 13, 2022

Conversation

emilyashley
Copy link
Contributor

@emilyashley emilyashley commented Sep 13, 2022 •
edited
Loading

Which problem is this PR solving?

Short description of the changes

  • Add node version to user-agent header.
  • Test expected versions are in the header and that it is in the correct order

Screen Shot 2022-09-13 at 12 25 05 PM

@emilyashley emilyashley marked this pull request as ready for review September 13, 2022 17:47
@emilyashley emilyashley requested review from a team and robbkidd September 13, 2022 17:47
@emilyashley emilyashley changed the title Add node to the user agent header. Add node/version to the user agent header. Sep 13, 2022
@emilyashley emilyashley self-assigned this Sep 13, 2022
@emilyashley emilyashley added type: maintenance The necessary chores to keep the dust off. version: bump patch A PR with release-worthy changes and is backwards-compatible. labels Sep 13, 2022
@robbkidd
Copy link
Member

Does the team have opinions on if it's node/v#.#.# or node/#.#.#

I wish for no-v (node/#.#.#), but we have other user-agents incoming with "v" in the version number which means we have to accommodate them anyway, so I'd say whichever you already have working.

pkanal
pkanal approved these changes Sep 13, 2022
View reviewed changes
Copy link
Contributor

@pkanal pkanal left a comment

Choose a reason for hiding this comment

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

Nice! I think we should keep the string to whatever process.version is

robbkidd
robbkidd approved these changes Sep 13, 2022
View reviewed changes
Copy link
Member

@robbkidd robbkidd left a comment

Choose a reason for hiding this comment

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

Join the JS feista!

src/__tests__/transmission_test.js
userAgent.indexOf("addition") === -1
},
{
dataset: "test-transmission2",
addition: "user-agent addition",
probe: userAgent =>
userAgent.indexOf("libhoney") === 0 &&
userAgent.indexOf("addition") !== -1
// user-agent order: libhoney, addition, node
Copy link
Member

Choose a reason for hiding this comment

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

✨ intention revealing commentary ✨

Terrific

@emilyashley emilyashley merged commit 28bf5a9 into main Sep 13, 2022
@emilyashley emilyashley deleted the emash-node-user-agent-header branch September 13, 2022 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pkanal pkanal pkanal approved these changes

@robbkidd robbkidd robbkidd approved these changes

Assignees

@emilyashley emilyashley

Labels
type: maintenance The necessary chores to keep the dust off. version: bump patch A PR with release-worthy changes and is backwards-compatible.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add node version to the user-agent header
3 participants
@emilyashley @robbkidd @pkanal