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.

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

src: sort internal binding list #24292

Merged
merged 1 commit into from
Nov 13, 2018
Merged

src: sort internal binding list #24292

merged 1 commit into from
Nov 13, 2018

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Nov 10, 2018

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

'util',
'uv',
'v8',
'zlib'
Copy link
Member

Choose a reason for hiding this comment

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

would it be possible to put a trailing comma here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possible, yes. But we don't currently have anything that would come after zlib.

@Trott
Copy link
Member

Trott commented Nov 12, 2018

Nit: commit message subsystem should likely be lib rather than src?

PR-URL: nodejs#24292
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Weijia Wang <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
@cjihrig
Copy link
Contributor Author

cjihrig commented Nov 13, 2018

@cjihrig cjihrig merged commit bda4643 into nodejs:master Nov 13, 2018
@cjihrig cjihrig deleted the sort branch November 13, 2018 04:28
BridgeAR pushed a commit that referenced this pull request Nov 14, 2018
PR-URL: #24292
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Weijia Wang <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
kiyomizumia pushed a commit to kiyomizumia/node that referenced this pull request Nov 15, 2018
PR-URL: nodejs#24292
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Weijia Wang <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
@codebytere
Copy link
Member

@cjihrig do you have time to backport this to 10.x?

@cjihrig
Copy link
Contributor Author

cjihrig commented Jan 13, 2019

I don't think this applies to 10.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.