Skip to content

lib: use primordials for navigator.userAgent - #50467

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
Uzlopak:primordials-useragent
Nov 2, 2023
Merged

lib: use primordials for navigator.userAgent#50467
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
Uzlopak:primordials-useragent

Conversation

@Uzlopak

@UzlopakUzlopak commented Oct 29, 2023

Copy link
Copy Markdown
Contributor

@anonrig
@ljharb

I got the PR comment, that we should use primordials for userAgent by ljharb As that line was not touched by my PR I want to mark it as resolved, but not lose the information. So here I provide the corresponding PR and resolve that remark in my PR ;)

@nodejs-github-bot

nodejs-github-bot commented Oct 29, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/web-standards

@nodejs-github-botnodejs-github-bot added the needs-ci PRs that need a full CI run. label Oct 29, 2023
@UzlopakUzlopak mentioned this pull request Oct 29, 2023
Closed
@GeoffreyBoothGeoffreyBooth added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Oct 29, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 29, 2023
@nodejs-github-bot

nodejs-github-bot commented Oct 29, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/55317/

anonrig
anonrig requested changes Oct 29, 2023
View reviewed changes

@anonriganonrig left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd like to get answer to my questions before merging this PR

Comment threadlib/internal/navigator.js
@anonriganonrig removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 29, 2023
@github-actionsgithub-actionsBot mentioned this pull request Oct 30, 2023
Open
25 tasks
tniessen
tniessen approved these changes Oct 30, 2023
View reviewed changes

@tniessentniessen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems like a constant that might as well be constructed before any user code runs, but LGTM either way.

@Uzlopak

Uzlopak commented Oct 30, 2023

Copy link
Copy Markdown
ContributorAuthor

What is the conclusion? :)
Can somebody please retrigger the failing ci?

@anonrig

anonrig commented Oct 30, 2023

Copy link
Copy Markdown
Member

What is the conclusion? :) Can somebody please retrigger the failing ci?

I've left a comment: #50467 (comment)

@github-actionsgithub-actionsBot mentioned this pull request Oct 31, 2023
Open
17 tasks
@nodejs-github-bot

nodejs-github-bot commented Oct 31, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/55337/

@nodejs-github-bot

nodejs-github-bot commented Oct 31, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/55335/

@nodejs-github-bot

nodejs-github-bot commented Oct 31, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/55338/

@github-actionsgithub-actionsBot mentioned this pull request Nov 1, 2023
Open
24 tasks
@UzlopakUzlopak mentioned this pull request Nov 1, 2023
Merged
@Uzlopak

Uzlopak commented Nov 1, 2023

Copy link
Copy Markdown
ContributorAuthor

Can somebody please have a look if the CI issues are unrelated and maybe merge?

@Uzlopak

Uzlopak commented Nov 2, 2023

Copy link
Copy Markdown
ContributorAuthor

@GeoffreyBooth
I think the failing tests are unrelated. Should I rebase from main?

@github-actionsgithub-actionsBot mentioned this pull request Nov 2, 2023
Open
25 tasks
@nodejs-github-bot

nodejs-github-bot commented Nov 2, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/55400/

@GeoffreyBooth

GeoffreyBooth commented Nov 2, 2023

Copy link
Copy Markdown
Member

Don't rebase unless you have to, we won't be able to resume CI once you do that.

@Uzlopak

Uzlopak commented Nov 2, 2023

Copy link
Copy Markdown
ContributorAuthor

Seems that the CI is green.

Can somebody add the author-ready label and/or merge it?

@H4adH4ad added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 2, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 2, 2023
@nodejs-github-bot
nodejs-github-bot merged commit 45f5c9b into nodejs:mainNov 2, 2023
@nodejs-github-bot

nodejs-github-bot commented Nov 2, 2023

Copy link
Copy Markdown
Collaborator

Landed in 45f5c9b

@Uzlopak
Uzlopak deleted the primordials-useragent branch November 2, 2023 13:33
@github-actionsgithub-actionsBot mentioned this pull request Nov 3, 2023
Open
22 tasks
targos pushed a commit that referenced this pull request Nov 11, 2023
@Uzlopak@targos
lib: use primordials for navigator.userAgent
4c02ec6
PR-URL: #50467
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
@targostargos mentioned this pull request Nov 12, 2023
Merged
targos pushed a commit that referenced this pull request Nov 14, 2023
@Uzlopak@targos
lib: use primordials for navigator.userAgent
c9bd0c5
PR-URL: #50467
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
@Uzlopak@UlisesGascon
lib: use primordials for navigator.userAgent
a114a91
PR-URL: #50467
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
@UlisesGasconUlisesGascon mentioned this pull request Dec 12, 2023
Merged
UlisesGascon pushed a commit that referenced this pull request Dec 13, 2023
@Uzlopak@UlisesGascon
lib: use primordials for navigator.userAgent
b677218
PR-URL: #50467
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
UlisesGascon pushed a commit that referenced this pull request Dec 15, 2023
@Uzlopak@UlisesGascon
lib: use primordials for navigator.userAgent
057e37c
PR-URL: #50467
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
UlisesGascon pushed a commit that referenced this pull request Dec 19, 2023
@Uzlopak@UlisesGascon
lib: use primordials for navigator.userAgent
48dbde7
PR-URL: #50467
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ljharbljharbljharb approved these changes
@GeoffreyBoothGeoffreyBoothGeoffreyBooth approved these changes
@benjamingrbenjamingrbenjamingr left review comments
@MoLowMoLowMoLow left review comments
@jasnelljasnelljasnell approved these changes
@anonriganonriganonrig approved these changes
@tniessentniessentniessen approved these changes
@H4adH4adH4ad approved these changes
@Ethan-ArrowoodEthan-ArrowoodEthan-Arrowood approved these changes

Assignees

No one assigned

Labels

needs-ciPRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

11 participants

@Uzlopak@nodejs-github-bot@anonrig@GeoffreyBooth@ljharb@jasnell@benjamingr@tniessen@MoLow@H4ad@Ethan-Arrowood