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.

Sign up for GitHub

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

Jump to bottom

alpha-sort sourcepaths in v8/unittests.gyp #25010

Closed
ryzokuken opened this issue Dec 13, 2018 · 3 comments
Closed

alpha-sort sourcepaths in v8/unittests.gyp #25010

ryzokuken opened this issue Dec 13, 2018 · 3 comments

Comments

@ryzokuken
Copy link
Contributor

@targos @refack while adding a new source for today's V8 upgrade, I noticed that at least one section in v8/unittests.gyp isn't sorted alphabetically. Do you think it's worthwhile to spend some time to sort the entire file? In the future, it will help us place and find sourcepaths easily.
@targos
Copy link
Member

targos commented Dec 13, 2018

Is unittests.gyp used by anything in Node?

@ryzokuken
Copy link
Contributor Author

@targos According to @refack (we talked about this in a private conversation):

AFAICT we only use v8.gyp features.gypi, toolset.gypi, inspector.gypi. And in some cases d8.gyp

In that case, I'll try removing all of the others and see if that fails. I think it's unnecessary on our end to maintain all these files if they're literally used nowhere.

targos added a commit to targos/node that referenced this issue Dec 17, 2018
@targos
deps: remove test-related GYP files
7ef5ff5 
- Less files to maintain.
- We don't use them.
- We build with GN to run V8 tests.

Fixes: nodejs#25095
Refs: nodejs#25010
@targos targos mentioned this issue Dec 17, 2018
Closed
targos added a commit that referenced this issue Dec 20, 2018
@targos
deps: remove test-related GYP files
f0e4609 
- Less files to maintain.
- We don't use them.
- We build with GN to run V8 tests.

Fixes: #25095
Refs: #25010

PR-URL: #25097
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
@bnoordhuis
Copy link
Member

#25097 landed so this can be closed, right? I'm closing it, please reopen if that's a mistake.

refack pushed a commit to refack/node that referenced this issue Jan 14, 2019
@targos @refack
deps: remove test-related GYP files
5ad4f19 
- Less files to maintain.
- We don't use them.
- We build with GN to run V8 tests.

Fixes: nodejs#25095
Refs: nodejs#25010

PR-URL: nodejs#25097
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@bnoordhuis @targos @ryzokuken