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

src: promote v8 name spaces with using by gireeshpunathil · Pull Request #22641 · nodejs/node · GitHub

/ node Public

src: promote v8 name spaces with using - #22641

Closed
gireeshpunathil wants to merge 1 commit into
nodejs:masterfrom
gireeshpunathil:namespace
Closed

src: promote v8 name spaces with using#22641
gireeshpunathil wants to merge 1 commit into
nodejs:masterfrom
gireeshpunathil:namespace

Conversation

Copy link
Copy Markdown
Member

there are several places where v8 artifacts appear with scope resolution
operator inline with the source. Elevate them for improved readability
as well as to follow the convention.

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

Copy link
Copy Markdown
Collaborator

nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Sep 1, 2018
Comment thread src/node.cc Outdated

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

The backslash is not aligned here.

You can run make format-cpp to autoformat your changes.

Copy link
Copy Markdown
Member Author

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

@targos - done, thanks. ptal.

Comment thread src/node.cc Outdated

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

Nit: fix spacing.

Copy link
Copy Markdown
Member Author

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

@TimothyGu - done, ptal, thanks!

there are several places where v8 artifacts appear with scope resolution
operator inline with the source. Elevate them for improved readability
as well as to follow the convention.

gireeshpunathil commented Sep 3, 2018
edited by addaleax
Loading

Copy link
Copy Markdown
Member Author

CI: https://ci.nodejs.org/job/node-test-pull-request/16983/ (:heavy_check_mark:)

danbev commented Sep 5, 2018

Copy link
Copy Markdown
Contributor

Landed in 31ca5dc.

danbev closed this Sep 5, 2018
danbev pushed a commit that referenced this pull request Sep 5, 2018
there are several places where v8 artifacts appear with scope resolution
operator inline with the source. Elevate them for improved readability
as well as to follow the convention.

PR-URL: #22641
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Sep 5, 2018
there are several places where v8 artifacts appear with scope resolution
operator inline with the source. Elevate them for improved readability
as well as to follow the convention.

PR-URL: #22641
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Sep 6, 2018
there are several places where v8 artifacts appear with scope resolution
operator inline with the source. Elevate them for improved readability
as well as to follow the convention.

PR-URL: #22641
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants


Back | FazBrowse Home | New Git URL