Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 1 commit into from
Oct 30, 2018
Merged

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

merged 1 commit into from
Oct 30, 2018
+3 −3

Conversation

oyyd
Copy link
Contributor

@oyyd oyyd commented Oct 26, 2018

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 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
Copy link
Member

trivikr commented Oct 29, 2018

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

@lundibundi lundibundi added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 29, 2018
@joyeecheung
Copy link
Member

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

@oyyd @refack
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 <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
@refack
Copy link
Contributor

refack commented Oct 30, 2018

Landed in 2cab511 🎉

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

PR-URL: #23894
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
@BridgeAR 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
@oyyd @MylesBorins
src: use "constants" string instead of creating new one
592f5bd 
Using the same "constants" string in c++.

PR-URL: #23894
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 27, 2018
@oyyd @MylesBorins
src: use "constants" string instead of creating new one
b1a2960 
Using the same "constants" string in c++.

PR-URL: #23894
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
@codebytere codebytere mentioned this pull request Nov 27, 2018
Closed
rvagg pushed a commit that referenced this pull request Nov 28, 2018
@oyyd @rvagg
src: use "constants" string instead of creating new one
e8dbd09 
Using the same "constants" string in c++.

PR-URL: #23894
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
@oyyd @rvagg
src: use "constants" string instead of creating new one
34dd0cd 
Using the same "constants" string in c++.

PR-URL: #23894
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 29, 2018
@oyyd @MylesBorins
src: use "constants" string instead of creating new one
d82e818 
Using the same "constants" string in c++.

PR-URL: #23894
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
@codebytere codebytere mentioned this pull request Nov 29, 2018
Merged
@BethGriggs BethGriggs mentioned this pull request Dec 4, 2018
Merged
This was referenced Dec 18, 2018
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trivikr trivikr trivikr approved these changes

@richardlau richardlau richardlau approved these changes

@cjihrig cjihrig cjihrig approved these changes

@targos targos targos approved these changes

@TimothyGu TimothyGu TimothyGu approved these changes

@lpinca lpinca lpinca approved these changes

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

@ChALkeR ChALkeR ChALkeR approved these changes

Assignees
No one assigned
Labels
author ready PRs 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++. 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.