Skip to content

Conversation

@mmomtchev
Copy link
Contributor

Add two tips for speeding up the dev builds

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

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. tls Issues and PRs related to the tls subsystem. labels Dec 9, 2020
BUILDING.md Outdated
Copy link
Member

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 ?

Copy link
Contributor Author

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
Copy link
Member

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)

Copy link
Member

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).

Copy link
Member

@benjamingr benjamingr left a 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

Copy link
Member

@RaisinTen RaisinTen left a comment

Choose a reason for hiding this comment

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

This is great! ❤️

Copy link
Member

@Trott Trott left a 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.

@Trott Trott closed this Dec 18, 2020
Trott pushed a commit that referenced this pull request Dec 18, 2020
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>
@Trott
Copy link
Member

Trott commented Dec 18, 2020

Landed in 36581f1

targos pushed a commit that referenced this pull request Dec 21, 2020
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>
targos pushed a commit that referenced this pull request May 1, 2021
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>
@danielleadams danielleadams mentioned this pull request May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. tls Issues and PRs related to the tls subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants