Skip to content

src:Elevate v8 namespaces of repeatedly referenced artifacts with - #24429

Closed
manilson wants to merge 1 commit into
nodejs:masterfrom
manilson:master
Closed

src:Elevate v8 namespaces of repeatedly referenced artifacts with#24429
manilson wants to merge 1 commit into
nodejs:masterfrom
manilson:master

Conversation

@manilson

@manilsonmanilson commented Nov 17, 2018

Copy link
Copy Markdown

'using' keyword in src/node_i18n.cc

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. i18n-api Issues and PRs related to the i18n implementation. labels Nov 17, 2018
@gireeshpunathilgireeshpunathil added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Nov 17, 2018
targos
targos reviewed Nov 17, 2018
View reviewed changes
Comment threadsrc/node_i18n.cc Outdated
@targos

targos commented Nov 17, 2018

Copy link
Copy Markdown
Member

The commit message should start with src: elevate ... and its first line should be shorter (max 50 characters)

refack
refack approved these changes Nov 17, 2018
View reviewed changes

@refackrefack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM % duplicate line

@refackrefack added the fast-track PRs that do not need to wait for 48 hours to land. label Nov 17, 2018
@refack

refack commented Nov 17, 2018

Copy link
Copy Markdown
Contributor

Please 👍 for fast-track (nits could be resolved during landing)

@refack

refack commented Nov 17, 2018

Copy link
Copy Markdown
Contributor

Lite CI:https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/

@Trott

Trott commented Nov 19, 2018

Copy link
Copy Markdown
Member

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

@refack

refack commented Nov 19, 2018

Copy link
Copy Markdown
Contributor

Hello @manilson and welcome. Thank you for your contribution 🥇
If you are not familiar with our review and landing process, it's covered in CONTRIBUTING.md

P.S. If you have any questions you can also feel free to contact me directly.

P.P.S. If you like you can associate your email address with you GitHub account. That way your contributions will be properly credited to you. See https://help.github.com/articles/setting-your-commit-email-address-on-github/

@refackrefack added blocked PRs that are blocked by other issues or PRs. and removed fast-track PRs that do not need to wait for 48 hours to land. labels Nov 19, 2018
@refack

refack commented Nov 19, 2018

Copy link
Copy Markdown
Contributor

blocking to give the OP a chance to set their email address.

@addaleaxaddaleax removed the blocked PRs that are blocked by other issues or PRs. label Nov 20, 2018
@addaleax

addaleax commented Nov 20, 2018

Copy link
Copy Markdown
Member

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

@addaleaxaddaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 20, 2018
@Trott

Trott commented Nov 20, 2018

Copy link
Copy Markdown
Member

Landed in 77c2cf9.

Thanks for the contribution! 🎉

(If you're interested in other possible contributions to Node.js but don't have a good idea of where to start looking, some ideas are posted at https://www.nodetodo.org/next-steps/.)

@TrottTrott closed this Nov 20, 2018
Trott pushed a commit to Trott/io.js that referenced this pull request Nov 20, 2018
Maya Anilson@Trott
src: elevate namespaces of repeated artifacts
77c2cf9
PR-URL: nodejs#24429
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
targos pushed a commit that referenced this pull request Nov 21, 2018
Maya Anilson@targos
src: elevate namespaces of repeated artifacts
6a5a50d
PR-URL: #24429
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
Maya Anilson@rvagg
src: elevate namespaces of repeated artifacts
b8ed930
PR-URL: #24429
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@BridgeARBridgeAR mentioned this pull request Dec 5, 2018
Merged
4 tasks
This was referenced Dec 7, 2018
Closed
Closed
This was referenced Dec 7, 2018
Closed
Closed
Closed
codebytere pushed a commit that referenced this pull request Jan 13, 2019
Maya Anilson@codebytere
src: elevate namespaces of repeated artifacts
ff4eeae
PR-URL: #24429
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
Maya Anilson@refack
src: elevate namespaces of repeated artifacts
2a68866
PR-URL: nodejs#24429
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@codebyterecodebytere mentioned this pull request Jan 15, 2019
Merged
codebytere pushed a commit that referenced this pull request Jan 29, 2019
Maya Anilson@codebytere
src: elevate namespaces of repeated artifacts
4b82aa8
PR-URL: #24429
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
This was referenced Jan 29, 2019
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@targostargostargos left review comments
@jasnelljasnelljasnell approved these changes
@gireeshpunathilgireeshpunathilgireeshpunathil approved these changes
+2 more reviewers
@refackrefackrefack approved these changes
@thefourtheyethefourtheyethefourtheye approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.c++Issues and PRs that require attention from people who are familiar with C++.code-and-learnIssues related to the Code-and-Learn events and PRs submitted during the events.i18n-apiIssues and PRs related to the i18n implementation.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@manilson@targos@refack@Trott@addaleax@jasnell@thefourtheye@gireeshpunathil@nodejs-github-bot