Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Nov 12, 2019

Refs: #29766

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

@nodejs-github-bot nodejs-github-bot added the net Issues and PRs related to the net subsystem. label Nov 12, 2019
@gireeshpunathil gireeshpunathil added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Nov 12, 2019
@ghost
Copy link
Author

ghost commented Nov 12, 2019

I'm actually getting a linter error on a line I didn't change (L538):

defineProperty(Socket.prototype, 'bufferSize', {
  get: function() { // eslint-disable-line getter-return
    if (this._handle) {
      return this[kLastWriteQueueSize] + this.writableLength;
    }
  }
});

Should I address that as part of this PR as well?

@lundibundi
Copy link
Member

Also, it looks like we usually go with capitalized Object... in such cases, could you please replace them objectSetPrototypeOf -> ObjectSetPrototypeOf and such? Sorry for the typo in the first comment.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@ghost
Copy link
Author

ghost commented Nov 18, 2019

Also, it looks like we usually go with capitalized Object... in such cases, could you please replace them objectSetPrototypeOf -> ObjectSetPrototypeOf and such?

Ooops, renamed. Thanks for bearing with me!

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

gireeshpunathil pushed a commit that referenced this pull request Nov 19, 2019
Refs: #29766
PR-URL: #30447
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@gireeshpunathil
Copy link
Member

landed in 5e4d99a, thanks for the contribution!

BridgeAR pushed a commit that referenced this pull request Nov 19, 2019
Refs: #29766
PR-URL: #30447
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@BridgeAR BridgeAR mentioned this pull request Nov 19, 2019
targos pushed a commit that referenced this pull request Dec 1, 2019
Refs: #29766
PR-URL: #30447
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Dec 9, 2019
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
Refs: #29766
PR-URL: #30447
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Dec 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. net Issues and PRs related to the net subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants