★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/29073Nouvelle recherche | Portail wanayoo
Skip to content

/node

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: organize imports in inspector_profiler.cc #29073

Closed
wants to merge 1 commit into from
+4 −3

Conversation

@pi1024e
Copy link
Contributor

commented Aug 9, 2019

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

In the other .cc files in the project, includes are in alphabetical order, with local files first, and libraries after. However, inspector_profiler.cc has a library declared in the middle of the import order, and v8 is the second to last being imported, instead of the last. So I reordered the imports and testing showed no side effects; everything passed.

It is a small change and it does not change any behavior. However, if you are weary of accepting this PR I understand.

@nodejs-github-bot nodejs-github-bot added C++ inspector labels Aug 9, 2019

@addaleax
addaleax approved these changes Aug 9, 2019
View changes
@nodejs-github-bot

This comment has been minimized.

Sign in to view
Copy link

commented Aug 9, 2019

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

@pi1024e pi1024e force-pushed the pi1024e:master branch 2 times, most recently from 1eef586 to 0f75d50 Aug 9, 2019

@eugeneo
eugeneo approved these changes Aug 9, 2019
View changes
@nodejs-github-bot

This comment has been minimized.

Sign in to view
Copy link

commented Aug 9, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/24909/
@Trott
Trott approved these changes Aug 9, 2019
View changes
@nodejs-github-bot

This comment has been minimized.

Sign in to view
Copy link

commented Aug 9, 2019

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

@pi1024e pi1024e force-pushed the pi1024e:master branch 2 times, most recently from a395d7e to b688058 Aug 10, 2019

@pi1024e pi1024e changed the title src: organize imports in inspector_profiler.cc to match import order of other files src: organize imports in inspector_profiler.cc Aug 10, 2019

@pi1024e pi1024e force-pushed the pi1024e:master branch from 9f78fca to f0fdb47 Aug 10, 2019

@ZYSzys
ZYSzys approved these changes Aug 10, 2019
View changes

@pi1024e pi1024e force-pushed the pi1024e:master branch from f0fdb47 to 85c0575 Aug 11, 2019

@pi1024e
src: organize imports in inspector_profiler.cc
Loading status checks…
fb173ae

@pi1024e pi1024e force-pushed the pi1024e:master branch from 12d4dbd to fb173ae Aug 11, 2019

@nodejs-github-bot

This comment has been minimized.

Sign in to view
Copy link

commented Aug 12, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/24943/
@Trott

This comment has been minimized.

Sign in to view
Copy link
Member

commented Aug 12, 2019

Landed in 15b2d13.

Thanks for the contribution! 🎉

@Trott Trott closed this Aug 12, 2019

Trott added a commit to Trott/io.js that referenced this pull request Aug 12, 2019
@pi1024e @Trott
src: organize imports in inspector_profiler.cc 
In the other .cc files in the project, includes are in alphabetical
order, with local files first, and libraries after. However,
inspector_profiler.cc has a library declared in the middle of the import
order, and v8 is the second to last being imported, instead of the last.
So I reordered the imports and testing showed no side effects;
everything passed.

PR-URL: nodejs#29073
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
15b2d13
targos added a commit that referenced this pull request Aug 19, 2019
@pi1024e @targos
src: organize imports in inspector_profiler.cc 
In the other .cc files in the project, includes are in alphabetical
order, with local files first, and libraries after. However,
inspector_profiler.cc has a library declared in the middle of the import
order, and v8 is the second to last being imported, instead of the last.
So I reordered the imports and testing showed no side effects;
everything passed.

PR-URL: #29073
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Verified
This commit was signed with a verified signature.
targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
ee2dd5f
targos added a commit that referenced this pull request Aug 19, 2019
@pi1024e @targos
src: organize imports in inspector_profiler.cc 
In the other .cc files in the project, includes are in alphabetical
order, with local files first, and libraries after. However,
inspector_profiler.cc has a library declared in the middle of the import
order, and v8 is the second to last being imported, instead of the last.
So I reordered the imports and testing showed no side effects;
everything passed.

PR-URL: #29073
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Verified
This commit was signed with a verified signature.
targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
b2936cf
@targos targos referenced this pull request Aug 19, 2019
Merged
This was referenced Aug 20, 2019
Closed
Closed
Closed
Closed
Closed
@beevelop beevelop referenced this pull request Aug 20, 2019
Closed
JeniaBR added a commit to JeniaBR/node that referenced this pull request Sep 11, 2019
@pi1024e @JeniaBR
src: organize imports in inspector_profiler.cc 
In the other .cc files in the project, includes are in alphabetical
order, with local files first, and libraries after. However,
inspector_profiler.cc has a library declared in the middle of the import
order, and v8 is the second to last being imported, instead of the last.
So I reordered the imports and testing showed no side effects;
everything passed.

PR-URL: nodejs#29073
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
8bd22d8
JeniaBR added a commit to JeniaBR/node that referenced this pull request Sep 11, 2019
@pi1024e @JeniaBR
src: organize imports in inspector_profiler.cc 
In the other .cc files in the project, includes are in alphabetical
order, with local files first, and libraries after. However,
inspector_profiler.cc has a library declared in the middle of the import
order, and v8 is the second to last being imported, instead of the last.
So I reordered the imports and testing showed no side effects;
everything passed.

PR-URL: nodejs#29073
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
11dc65b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eugeneo eugeneo

@Trott Trott

@addaleax addaleax

@ZYSzys ZYSzys

Assignees
No one assigned
Labels
C++ inspector
Projects
None yet
Milestone
No milestone
6 participants
@pi1024e @nodejs-github-bot @Trott @eugeneo @addaleax @ZYSzys
You can’t perform that action at this time.