GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — doc: document AliasedBuffers usage scope in style guide by gireeshpunathil · Pull Request #24724 · nodejs/node

Conversation

@gireeshpunathil

@gireeshpunathil gireeshpunathil commented Nov 29, 2018

Copy link
Copy Markdown
Member

Expose AliasedBuffer API and its function through the C++ style guide, and also differentiate it with
TypedArrays.

Fixes: #22977

A draft attempt to follow @joyeecheung 's thought, open to iterate over the verbiage.

/cc @joyeecheung @addaleax

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@nodejs-github-bot

nodejs-github-bot commented Nov 29, 2018

Copy link
Copy Markdown
Collaborator

@gireeshpunathil build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1725/pipeline
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to Node.js documentation. label Nov 29, 2018
@gireeshpunathil
gireeshpunathil requested review from addaleax, cjihrig and joyeecheung November 29, 2018 14:28
targos
targos reviewed Nov 29, 2018
View reviewed changes
Comment thread CPP_STYLE_GUIDE.md Outdated

@targos targos Nov 29, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
AliasedBuffer on the other had, provides direct access to the backing
AliasedBuffer on the other hand, provides direct access to the backing

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@addaleax

addaleax commented Nov 29, 2018

Copy link
Copy Markdown
Member

Um … this reads a bit confusing to me. AliasedBuffers are TypedArrays, and conversely, we can directly access a TypedArray’s memory? I’m not sure there’s anything to actually choose here…

The last paragraph also seems somewhat odd, because the reason that we have AliasedBuffer is that we can use the abstraction to hook into the VM’s object modification observation mechanisms; so it seems like the opposite is true? Not using AliasedBuffer to modify TypedArray memory is what breaks these mechanisms. (This detection stuff is only actually supported by @nodejs/chakracore, afaik, and I’m not sure about the state of that project as a whole.)

So: We should use AliasedBuffers when we want the VM to be able to observe modifications from the C++ side. That’s pretty it, as far as I know.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vsemozhetbyt
vsemozhetbyt reviewed Nov 29, 2018
View reviewed changes
Comment thread CPP_STYLE_GUIDE.md Outdated

@vsemozhetbyt vsemozhetbyt Nov 29, 2018

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
* [Choice between Typed Arrays and AliasedBuffers](#typed-arrays-andaliased-buffers)
* [Choice between Typed Arrays and AliasedBuffers](#choice-between-typed-arrays-and-aliasedbuffers)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
vsemozhetbyt
vsemozhetbyt reviewed Nov 29, 2018
View reviewed changes
Comment thread CPP_STYLE_GUIDE.md Outdated

@vsemozhetbyt vsemozhetbyt Nov 29, 2018

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
these APIs are straightforward, the data created through these APIs
these APIs is straightforward, the data created through these APIs

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
vsemozhetbyt
vsemozhetbyt reviewed Nov 29, 2018
View reviewed changes
Comment thread CPP_STYLE_GUIDE.md Outdated

@vsemozhetbyt vsemozhetbyt Nov 29, 2018

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
becomes visible through the wrapping JS object and vice versa.
become visible through the wrapping JS object and vice versa.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
vsemozhetbyt
vsemozhetbyt reviewed Nov 29, 2018
View reviewed changes
Comment thread CPP_STYLE_GUIDE.md Outdated

@vsemozhetbyt vsemozhetbyt Nov 29, 2018 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
While this is more efficient than Typed Arrays, it overrides the v8's
While this is more efficient than Typed Arrays, it overrides the V8's

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@gireeshpunathil

gireeshpunathil commented Nov 30, 2018

Copy link
Copy Markdown
Member Author

thanks for the reviews! let me wait to hear from @joyeecheung as well.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@joyeecheung

joyeecheung commented Nov 30, 2018 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

I agree with @addaleax . For the background of the AliasedBuffer abstraction, see #15077

The most applicable case of AliasedBuffer is when you have some global data that you need to pass between C++ and JS out-of-band. For example, the AliasedBuffers on the Environment fs_stats_field_array are globally shared among all the fs.*stat callback wrappers and fs.*statSync implementation, the C++ bindings put the results in those buffers and the JS code would read them with the assumption that during the execution the data in those global buffers are fresh and are exactly what they want. This makes it difficult for ChakraCore to implement the time-travel debugging because there is no way for the VM to observe this mutation, hence the abstraction was introduced. Although, AFAICT, we now always return the buffer or pass it to callbacks before we return to the JS layer, and the JS layer does not actually grab it from the global binding anymore (even though essentially the values are still global per environment, we only pass around a local reference to that global), so in theory the VM should be able to observe the change without this abstraction now.

They are also used when the ArrayBufferViews need to be persistent, e.g. the ones in the FSReqPromise - those are not globally shared among callsites, but they are globals to the V8 isolate, and can be garbage collected after the promise resolves and the JS land is done with them . Although these are also always resolved back to the JS land now so I guess it's not strictly necessary for the VM to observe them through the abstraction, but nonetheless AliasedBuffer serves as a home-grown abstraction for manipulating these global arrays so there is still merit in it.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gireeshpunathil
gireeshpunathil force-pushed the cppguide branch from 062d47c to 240df6e Compare November 30, 2018 14:15
@gireeshpunathil

gireeshpunathil commented Nov 30, 2018

Copy link
Copy Markdown
Member Author

thanks for all the reviews! pushed a modified version that I think covers the essence of inputs especially from @addaleax and @joyeecheung ; PTAL, thanks!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
addaleax
addaleax reviewed Nov 30, 2018
View reviewed changes
Comment thread CPP_STYLE_GUIDE.md Outdated

@addaleax addaleax Nov 30, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We usually use the [] accessors, so I think the style guide should list them as the primary alternative?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread CPP_STYLE_GUIDE.md Outdated

@addaleax addaleax Nov 30, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This paragraph is very hard to read for me.

I don’t want to tell you what to write, but I think something like this would do:

When modifying typed arrays directly from C++, use an AliasedBuffer when possible,
because that makes it easier for some JS debuggers to observe these modifications.

We’d probably want to go into more detail when – if ever – something like node-chakracore gets merged into Node.js core, and we actually need to take care of always making modifications observable. For now, the chakracore team has been doing that on their own in the places where we don’t use AliasedBuffer.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gireeshpunathil gireeshpunathil Dec 1, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@addaleax - thanks, pushed a modified version with almost your suggested wordings. ptal, thanks.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@gireeshpunathil gireeshpunathil changed the title doc: document choice between TypedArray and AliasedBuffer in style guide doc: document AliasedBuffers usage scope in style guide Dec 1, 2018
addaleax
addaleax reviewed Dec 1, 2018
View reviewed changes
Comment thread CPP_STYLE_GUIDE.md Outdated

@addaleax addaleax Dec 1, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think that this provides placeholders is only relevant to VM engine implementers (i.e. not Node.js coders), so I don’t think it needs to be in the style guide.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@addaleax addaleax Dec 1, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

(This information is also included in the doc comment for AliasedBuffer, which is a much better place for it anyway?)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gireeshpunathil gireeshpunathil Dec 2, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

ok, removed the comment that reveals info relevant to VM implementors.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread CPP_STYLE_GUIDE.md Outdated

@addaleax addaleax Dec 1, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sorry, but I’m still somewhat confused… What does

when such modifications can have side effects outside of the callsite

mean?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gireeshpunathil gireeshpunathil Dec 2, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I mean to express this:
If the C++ layer modification of data is unanticipated / unforeseen / invisible at the callsite.

Example:

buffer.fill('x')

here, the backend buffer is filled with data at C++ level, and this is evident by looking at the JS callsite. Whereas:

obj.registerModule('foo)';

running into C++ land, and modifying artifacts linked to obj is not evident at the callsite.

My way of telling is to recommend AliasedBuffer usage in second type of cases. Does it make any meaning to you?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@addaleax addaleax Dec 2, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@gireeshpunathil Yes, but … the thing is, afaik (and you or @nodejs/chakracore can correct me if I’m wrong), there’s no real difference between these cases.

The VM debugger wouldn’t be notified of modifications even in the buffer.fill('x') case; and because we don’t use AliasedBuffer (and we can’t really, unless we port memset/memcpy to it), they still have to do explicit notifications after our modifications:

https://github.com/nodejs/node-chakracore/blob/38520056313bc747e3ca6dcc7598c2e51394c0ac/src/node_buffer.cc#L575-L583

(They haven’t fully covered all the Buffer methods, to be fair.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gireeshpunathil gireeshpunathil Dec 2, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@addaleax - ok; I removed that phrase to avoid confusion.

[On a side note]: In a sufficiently large application, what the VM (or a monitoring service that leverages the VM function) observed data at this level will be useful for? In other words, for a program or a user, a buffer.fill is an obvious callsite that mutates the buffer data at some lower levels, so may not be of real interest (or of any use), but would be interested to get notified on an out-of-bounds access because there is no normal way of knowing it otherwise?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@addaleax addaleax Dec 2, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@gireeshpunathil So, the ChakraCore application for this whole notification thingy is time-travel debugging… I haven’t ever tried that myself, but I believe the purpose here is to be able to compare the values of JS objects at different points in time, and that’s what the notifications are for? Without this, I think the debugger could show the same contents of a buffer after a .fill() call and when going back in time to before that call?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gireeshpunathil reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
addaleax
addaleax approved these changes Dec 2, 2018
View reviewed changes
@gireeshpunathil

gireeshpunathil commented Dec 2, 2018

Copy link
Copy Markdown
Member Author

CI: https://ci.nodejs.org/job/node-test-pull-request/19132/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
joyeecheung
joyeecheung reviewed Dec 2, 2018
View reviewed changes
Comment thread CPP_STYLE_GUIDE.md Outdated

@joyeecheung joyeecheung Dec 2, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Maybe Use AliasedBuffers to manipulate TypedArrays would be easier to recognize in the TOC?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gireeshpunathil gireeshpunathil Dec 2, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@joyeecheung - done, thanks.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread CPP_STYLE_GUIDE.md Outdated

@joyeecheung joyeecheung Dec 2, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The term some JS debuggers reads somewhat strange to me, can we just omit the reason and direct the reader to the comment in aliased_buffer.h? (one needs to read the header to use it anyway)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gireeshpunathil gireeshpunathil Dec 2, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

modified accordingly, and linked with the header file section for reference. Hope @addaleax is good with this change?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@gireeshpunathil
doc: add a note on usage scope of AliasedBuffer
1917213 
Explain usage context and scope of AliasedBuffer API and its
function in the C++ style guide. Provide an example code.

Fixes: nodejs#22977
@gireeshpunathil
gireeshpunathil force-pushed the cppguide branch from def2c93 to 1917213 Compare December 2, 2018 10:45
joyeecheung
joyeecheung approved these changes Dec 2, 2018
View reviewed changes
@gireeshpunathil

gireeshpunathil commented Dec 2, 2018

Copy link
Copy Markdown
Member Author

@targos @vsemozhetbyt the content you have reviewed and change suggested is fundamentally changed, so please take another look, thanks!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vsemozhetbyt
vsemozhetbyt approved these changes Dec 2, 2018
View reviewed changes

@vsemozhetbyt vsemozhetbyt left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I am not sure about these suggestions, so it would be good to get a +1 from native speakers and C++ users. Otherwise, doc format LGTM.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread CPP_STYLE_GUIDE.md Outdated

### Use AliasedBuffers to manipulate TypedArrays

When working with typed arrays that involves direct data modification

@vsemozhetbyt vsemozhetbyt Dec 2, 2018

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
When working with typed arrays that involves direct data modification
When working with typed arrays that involve direct data modification

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gireeshpunathil gireeshpunathil Dec 2, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

changed, thanks.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread CPP_STYLE_GUIDE.md Outdated
### Use AliasedBuffers to manipulate TypedArrays

When working with typed arrays that involves direct data modification
from C++, use an AliasedBuffer when possible. The API abstraction and

@vsemozhetbyt vsemozhetbyt Dec 2, 2018

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
from C++, use an AliasedBuffer when possible. The API abstraction and
from C++, use an `AliasedBuffer` when possible. The API abstraction and

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gireeshpunathil gireeshpunathil Dec 2, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

fixed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread CPP_STYLE_GUIDE.md Outdated

When working with typed arrays that involves direct data modification
from C++, use an AliasedBuffer when possible. The API abstraction and
the usage scope of AliasedBuffer is documented in [aliased_buffer.h][].

@vsemozhetbyt vsemozhetbyt Dec 2, 2018

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
the usage scope of AliasedBuffer is documented in [aliased_buffer.h][].
the usage scope of `AliasedBuffer` are documented in [aliased_buffer.h][].

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gireeshpunathil gireeshpunathil Dec 2, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

fixed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread CPP_STYLE_GUIDE.md Outdated
the usage scope of AliasedBuffer is documented in [aliased_buffer.h][].

```c++
// Create an AliasedBuffer

@vsemozhetbyt vsemozhetbyt Dec 2, 2018

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
// Create an AliasedBuffer
// Create an AliasedBuffer.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gireeshpunathil gireeshpunathil Dec 2, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

done.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@gireeshpunathil

gireeshpunathil commented Dec 2, 2018

Copy link
Copy Markdown
Member Author

@vsemozhetbyt thanks for the suggestion, and the recommendation. /cc @Trott (a native speaker) and @jasnell (a C++ user / both) - request a review!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gireeshpunathil
fixup: apply style nits
9960d5e
vsemozhetbyt
vsemozhetbyt reviewed Dec 2, 2018
View reviewed changes
Comment thread CPP_STYLE_GUIDE.md Outdated

When working with typed arrays that involve direct data modification
from C++, use an `AliasedBuffer` when possible. The API abstraction and
the usage scope of `AliasedBuffer` is documented in [aliased_buffer.h][].

@vsemozhetbyt vsemozhetbyt Dec 2, 2018

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

One more nit:

Suggested change
the usage scope of `AliasedBuffer` is documented in [aliased_buffer.h][].
the usage scope of `AliasedBuffer` are documented in [aliased_buffer.h][].

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gireeshpunathil gireeshpunathil Dec 3, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@vsemozhetbyt - changed, thanks.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@gireeshpunathil
fixup: grammar fix
ec0b5f6
addaleax
addaleax approved these changes Dec 3, 2018
View reviewed changes

@addaleax addaleax left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Still LGTM, yes :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@gireeshpunathil

gireeshpunathil commented Dec 4, 2018

Copy link
Copy Markdown
Member Author

scheduled CI: https://ci.nodejs.org/job/node-test-pull-request/19195
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Trott

Trott commented Dec 4, 2018

Copy link
Copy Markdown
Member

Landed in 447b390
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Trott Trott closed this Dec 4, 2018
Trott pushed a commit to Trott/io.js that referenced this pull request Dec 4, 2018
@gireeshpunathil @Trott
doc: add a note on usage scope of AliasedBuffer
447b390 
Explain usage context and scope of AliasedBuffer API and its
function in the C++ style guide. Provide an example code.

Fixes: nodejs#22977

PR-URL: nodejs#24724
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
@joyeecheung joyeecheung mentioned this pull request Dec 5, 2018
CI failures: 20181201-20181205 nodejs/reliability#19
Closed
BridgeAR pushed a commit that referenced this pull request Dec 5, 2018
@gireeshpunathil @BridgeAR
doc: add a note on usage scope of AliasedBuffer
3d1853b 
Explain usage context and scope of AliasedBuffer API and its
function in the C++ style guide. Provide an example code.

Fixes: #22977

PR-URL: #24724
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
@BridgeAR BridgeAR mentioned this pull request Dec 5, 2018
v11.4.0 proposal #24854
Merged
4 tasks done
This was referenced Dec 7, 2018
2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#261
Closed
2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#262
Closed
2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#263
Closed
2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#264
Closed
2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#265
Closed
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
@gireeshpunathil @refack
doc: add a note on usage scope of AliasedBuffer
654a0c9 
Explain usage context and scope of AliasedBuffer API and its
function in the C++ style guide. Provide an example code.

Fixes: nodejs#22977

PR-URL: nodejs#24724
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 12, 2019
@gireeshpunathil @BethGriggs
doc: add a note on usage scope of AliasedBuffer
7bd7328 
Explain usage context and scope of AliasedBuffer API and its
function in the C++ style guide. Provide an example code.

Fixes: #22977

PR-URL: #24724
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Feb 12, 2019
v10.15.3 proposal #26063
Merged
BethGriggs pushed a commit that referenced this pull request Feb 20, 2019
@gireeshpunathil @BethGriggs
doc: add a note on usage scope of AliasedBuffer
4f71fa1 
Explain usage context and scope of AliasedBuffer API and its
function in the C++ style guide. Provide an example code.

Fixes: #22977

PR-URL: #24724
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
@gireeshpunathil @rvagg
doc: add a note on usage scope of AliasedBuffer
daaf3a4 
Explain usage context and scope of AliasedBuffer API and its
function in the C++ style guide. Provide an example code.

Fixes: #22977

PR-URL: #24724
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleax addaleax addaleax approved these changes
@joyeecheung joyeecheung joyeecheung approved these changes
@cjihrig cjihrig Awaiting requested review from cjihrig
+2 more reviewers
@targos targos targos left review comments
@vsemozhetbyt vsemozhetbyt vsemozhetbyt approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

doc Issues and PRs related to Node.js documentation.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

doc: document AliasedBuffer in C++ style guide

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@gireeshpunathil @nodejs-github-bot @addaleax @joyeecheung @Trott @targos @vsemozhetbyt
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.