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

src: use "constants" string instead of creating new one by oyyd · Pull Request #23894 · nodejs/node · GitHub

/ node Public

src: use "constants" string instead of creating new one - #23894

Merged
refack merged 1 commit into
nodejs:masterfrom
oyyd:cpp-constants
Oct 30, 2018
ConversationCommits (1)ChecksFiles changed
Merged

src: use "constants" string instead of creating new one#23894
refack merged 1 commit into
nodejs:masterfrom
oyyd:cpp-constants

Conversation

oyyd commented Oct 26, 2018

Copy link
Copy Markdown
Contributor

I think it's ok to create them separately but it would be better if we use the same one especially when they have the exact same semantics.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Oct 26, 2018

trivikr commented Oct 29, 2018

Copy link
Copy Markdown
Member

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

lundibundi added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Oct 29, 2018

joyeecheung commented Oct 30, 2018

Copy link
Copy Markdown
Member

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

src: use "constants" string instead of creating new one
2cab511 
Using the same "constants" string in c++.

PR-URL: nodejs#23894
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>

refack commented Oct 30, 2018

Copy link
Copy Markdown
Contributor

Landed in 2cab511 🎉

refack merged commit 2cab511 into nodejs:master Oct 30, 2018
targos pushed a commit that referenced this pull request Nov 2, 2018
src: use "constants" string instead of creating new one
063b40e 
Using the same "constants" string in c++.

PR-URL: #23894
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
BridgeAR mentioned this pull request Nov 14, 2018
Merged
This was referenced Nov 15, 2018
Closed
Closed
Closed
Closed
Closed
Closed
MylesBorins pushed a commit that referenced this pull request Nov 27, 2018
src: use "constants" string instead of creating new one
592f5bd 
Using the same "constants" string in c++.

PR-URL: #23894
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 27, 2018
src: use "constants" string instead of creating new one
b1a2960 
Using the same "constants" string in c++.

PR-URL: #23894
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
codebytere mentioned this pull request Nov 27, 2018
Closed
rvagg pushed a commit that referenced this pull request Nov 28, 2018
src: use "constants" string instead of creating new one
e8dbd09 
Using the same "constants" string in c++.

PR-URL: #23894
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
src: use "constants" string instead of creating new one
34dd0cd 
Using the same "constants" string in c++.

PR-URL: #23894
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 29, 2018
src: use "constants" string instead of creating new one
d82e818 
Using the same "constants" string in c++.

PR-URL: #23894
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
codebytere mentioned this pull request Nov 29, 2018
Merged
BethGriggs mentioned this pull request Dec 4, 2018
Merged
This was referenced Dec 18, 2018
Closed
Closed
Closed
Closed
Closed
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

ChALkeR ChALkeR approved these changes
jasnell jasnell approved these changes
addaleax addaleax approved these changes
lpinca lpinca approved these changes
cjihrig cjihrig approved these changes
richardlau richardlau approved these changes
trivikr trivikr approved these changes
+2 more reviewers
TimothyGu TimothyGu approved these changes
targos targos approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Back | FazBrowse Home | New Git URL