Skip to content

src: refactor grow_async_ids_stack#23808

Merged
refack merged 2 commits intonodejs:masterfrom
refack:grow_async_ids
Oct 24, 2018
Merged

src: refactor grow_async_ids_stack#23808
refack merged 2 commits intonodejs:masterfrom
refack:grow_async_ids

Conversation

@refack
Copy link
Copy Markdown
Contributor

@refack refack commented Oct 21, 2018 •
edited
Loading

Added AliasedBuffer::reserve()

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

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-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Oct 21, 2018

@refack build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1327/pipeline

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Oct 21, 2018
@refack refack mentioned this pull request Oct 21, 2018
Merged
2 tasks
@refack refack self-assigned this Oct 21, 2018
joyeecheung
joyeecheung reviewed Oct 23, 2018
View reviewed changes
Comment thread src/aliased_buffer.h Outdated
Comment thread src/aliased_buffer.h Outdated
addaleax
addaleax reviewed Oct 23, 2018
View reviewed changes
Comment thread src/aliased_buffer.h Outdated
Copy link
Copy Markdown
Member

@addaleax addaleax Oct 22, 2018

Choose a reason for hiding this comment

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

style nit: old_size_in_bytes, new_size_in_bytes

Copy link
Copy Markdown
Contributor Author

@refack refack Oct 23, 2018

Choose a reason for hiding this comment

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

Ack.
Turned up the level of my IDE's inspection of these to "error".

Comment thread src/aliased_buffer.h Outdated
Copy link
Copy Markdown
Member

@addaleax addaleax Oct 22, 2018

Choose a reason for hiding this comment

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

style nit: NativeT* new_buffer

Copy link
Copy Markdown
Contributor Author

@refack refack Oct 23, 2018

Choose a reason for hiding this comment

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

Ack

joyeecheung
joyeecheung approved these changes Oct 23, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@joyeecheung joyeecheung left a comment

Choose a reason for hiding this comment

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

LGTM with the style nits from @addaleax

@refack
Copy link
Copy Markdown
Contributor Author

refack commented Oct 23, 2018

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

@mmarchini mmarchini added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 24, 2018
@refack refack force-pushed the grow_async_ids branch 2 times, most recently from 888b26e to b845fd6 Compare October 24, 2018 16:18
@refack
Copy link
Copy Markdown
Contributor Author

refack commented Oct 24, 2018

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

refack added 2 commits October 24, 2018 16:34
@refack
src: add AliasedBuffer::reserve
4044b06 
refactor grow_async_ids_stack

PR-URL: nodejs#23808
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
@refack
src: add direct dependency on *-inl.h file
586daae 
PR-URL: nodejs#23808
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
@refack refack merged commit 586daae into nodejs:master Oct 24, 2018
@refack refack deleted the grow_async_ids branch October 24, 2018 21:37
@refack refack removed their assignment Oct 24, 2018
targos pushed a commit that referenced this pull request Oct 26, 2018
@refack @targos
src: add AliasedBuffer::reserve
98ffcde 
refactor grow_async_ids_stack

PR-URL: #23808
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
targos pushed a commit that referenced this pull request Oct 26, 2018
@refack @targos
src: add direct dependency on *-inl.h file
2e7189b 
PR-URL: #23808
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
@targos targos mentioned this pull request Oct 27, 2018
Merged
@addaleax addaleax mentioned this pull request Oct 29, 2018
Closed
@targos
Copy link
Copy Markdown
Member

targos commented Nov 1, 2018

Marking dont-land-on-v11.x until nodejs/build#1542 is fixed

@refack
Copy link
Copy Markdown
Contributor Author

refack commented Nov 1, 2018

@targos there are two more PR that will require GCC4.9

@targos targos mentioned this pull request Nov 2, 2018
Closed
2 tasks
@refack
Copy link
Copy Markdown
Contributor Author

refack commented Nov 15, 2018

@targos the arm7 issue in ci-release was resolved - https://ci-release.nodejs.org/job/iojs+release/nodes=centos7-arm64-gcc6/3919/

targos pushed a commit that referenced this pull request Nov 18, 2018
@refack @targos
src: add AliasedBuffer::reserve
d0ab9d2 
refactor grow_async_ids_stack

PR-URL: #23808
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
targos pushed a commit that referenced this pull request Nov 18, 2018
@refack @targos
src: add direct dependency on *-inl.h file
a8ccf47 
PR-URL: #23808
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
@targos
Copy link
Copy Markdown
Member

targos commented Nov 18, 2018

@refack thanks! I backported this (and others) to v11.x and confirmed: https://ci-release.nodejs.org/job/iojs+release/3934/nodes=centos7-arm64-gcc6/

rvagg pushed a commit that referenced this pull request Nov 28, 2018
@refack @rvagg
src: add AliasedBuffer::reserve
33e7f6e 
refactor grow_async_ids_stack

PR-URL: #23808
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
@refack @rvagg
src: add direct dependency on *-inl.h file
25a9eee 
PR-URL: #23808
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
@BridgeAR BridgeAR mentioned this pull request Dec 5, 2018
Merged
4 tasks
This was referenced Dec 7, 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

@addaleax addaleax addaleax left review comments

@joyeecheung joyeecheung joyeecheung approved these changes

+1 more reviewer

@mmarchini mmarchini mmarchini 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 pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@refack @nodejs-github-bot @targos @addaleax @mmarchini @joyeecheung @codebytere