FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

lib: use primordials for navigator.userAgent by Uzlopak · Pull Request #50467 · nodejs/node · GitHub

/ node Public

lib: use primordials for navigator.userAgent - #50467

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
Uzlopak:primordials-useragent
Nov 2, 2023
ConversationCommits (1)ChecksFiles changed
Merged

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

Conversation

Uzlopak 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 commented Oct 29, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/web-standards

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Oct 29, 2023
Uzlopak mentioned this pull request Oct 29, 2023
Closed
GeoffreyBooth added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Oct 29, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 29, 2023

nodejs-github-bot commented Oct 29, 2023

Copy link
Copy Markdown
Collaborator

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

anonrig requested changes Oct 29, 2023
View reviewed changes

anonrig 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Comment thread lib/internal/navigator.js
anonrig removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Oct 29, 2023
github-actions Bot mentioned this pull request Oct 30, 2023
Open
25 tasks
tniessen approved these changes Oct 30, 2023
View reviewed changes

tniessen 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Uzlopak commented Oct 30, 2023

Copy link
Copy Markdown
Contributor Author

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

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-actions Bot mentioned this pull request Oct 31, 2023
Open
17 tasks

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 commented Oct 31, 2023

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Oct 31, 2023

Copy link
Copy Markdown
Collaborator

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

github-actions Bot mentioned this pull request Nov 1, 2023
Open
24 tasks
Uzlopak mentioned this pull request Nov 1, 2023
Merged

Uzlopak commented Nov 1, 2023

Copy link
Copy Markdown
Contributor Author

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

Uzlopak commented Nov 2, 2023

Copy link
Copy Markdown
Contributor Author

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

github-actions Bot mentioned this pull request Nov 2, 2023
Open
25 tasks

nodejs-github-bot commented Nov 2, 2023

Copy link
Copy Markdown
Collaborator

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

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 commented Nov 2, 2023

Copy link
Copy Markdown
Contributor Author

Seems that the CI is green.

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

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

nodejs-github-bot commented Nov 2, 2023

Copy link
Copy Markdown
Collaborator

Landed in 45f5c9b

Uzlopak deleted the primordials-useragent branch November 2, 2023 13:33
github-actions Bot mentioned this pull request Nov 3, 2023
Open
22 tasks
targos pushed a commit that referenced this pull request Nov 11, 2023
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>
targos mentioned this pull request Nov 12, 2023
Merged
targos pushed a commit that referenced this pull request Nov 14, 2023
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
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>
UlisesGascon mentioned this pull request Dec 12, 2023
Merged
UlisesGascon pushed a commit that referenced this pull request Dec 13, 2023
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
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
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

ljharb ljharb approved these changes
GeoffreyBooth GeoffreyBooth approved these changes
benjamingr benjamingr left review comments
MoLow MoLow left review comments
jasnell jasnell approved these changes
anonrig anonrig approved these changes
tniessen tniessen approved these changes
H4ad H4ad approved these changes
Ethan-Arrowood Ethan-Arrowood approved these changes

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Back | FazBrowse Home | New Git URL