-
-
Notifications
You must be signed in to change notification settings - Fork 34.1k
Add two tips for speeding up the dev builds #36452
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
Conversation
b7f8758 to
af1427d
Compare
BUILDING.md
Outdated
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.
Probably a good idea to include how to install ccache on max and windows as well in a comment?
brew install ccache
choco install ccache
And maybe link to https://ccache.dev/download.html ?
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.
OSX is flawless, but support for Visual Studio is far from perfect, I don't know if we should recommend to people to use it?
BUILDING.md
Outdated
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 would mention that in windows this is set and not export for the windows users?
(To be fair I've never used ccache in windows)
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 don't think ccache officially works with VS on Windows. I know in the CI we don't use it (we use clcache instead https://github.com/nodejs/build/blob/36202a24f7d19729bea1e0efb0fe7045b2c42dc6/ansible/roles/visual-studio/tasks/main.yml#L27-L54).
benjamingr
left a comment
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.
Good tips, left some nits
RaisinTen
left a comment
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 is great! ❤️
Trott
left a comment
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.
Left a comment about the anchor format but that's easy to fix. I agree that more can be done to show how to make it work in non apt-get environments, but those can be subsequent PRs. I'm OK with "I'm only PR'ing in the platforms I actually use a lot." This is useful as is and I'm totally OK if someone wants to land it.
7f96f9f to
36581f1
Compare
Add two important tips for novice Node.js contributors PR-URL: #36452 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Landed in 36581f1 |
Add two important tips for novice Node.js contributors PR-URL: #36452 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Add two important tips for novice Node.js contributors PR-URL: #36452 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Add two tips for speeding up the dev builds
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes