FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add two tips for speeding up the dev builds by mmomtchev · Pull Request #36452 · nodejs/node · GitHub

/ node Public

Add two tips for speeding up the dev builds - #36452

Closed
mmomtchev wants to merge 0 commit into
nodejs:masterfrom
mmomtchev:doc-fast-build
ConversationCommits (0)ChecksFiles changed
Closed

Add two tips for speeding up the dev builds#36452
mmomtchev wants to merge 0 commit into
nodejs:masterfrom
mmomtchev:doc-fast-build

Conversation

mmomtchev commented Dec 9, 2020

Copy link
Copy Markdown
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 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
gireeshpunathil reviewed Dec 9, 2020
View reviewed changes
Comment thread BUILDING.md Outdated
gireeshpunathil reviewed Dec 9, 2020
View reviewed changes
Comment thread BUILDING.md Outdated
benjamingr reviewed Dec 9, 2020
View reviewed changes
Comment thread BUILDING.md Outdated

benjamingr Dec 9, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 ?

mmomtchev Dec 11, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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?

benjamingr reviewed Dec 9, 2020
View reviewed changes
Comment thread BUILDING.md Outdated

benjamingr Dec 9, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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)

richardlau Dec 9, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 approved these changes Dec 9, 2020
View reviewed changes

benjamingr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Good tips, left some nits

RaisinTen reviewed Dec 12, 2020
View reviewed changes
Comment thread BUILDING.md Outdated
RaisinTen reviewed Dec 12, 2020
View reviewed changes
Comment thread BUILDING.md Outdated
RaisinTen approved these changes Dec 12, 2020
View reviewed changes

RaisinTen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This is great! ❤️

Trott reviewed Dec 14, 2020
View reviewed changes
Comment thread BUILDING.md Outdated
Trott approved these changes Dec 14, 2020
View reviewed changes

Trott left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 closed this Dec 18, 2020

Trott commented Dec 18, 2020

Copy link
Copy Markdown
Member

Landed in 36581f1

targos mentioned this pull request Dec 22, 2020
Merged
danielleadams mentioned this pull request May 3, 2021
Merged
snyk-bot mentioned this pull request Sep 13, 2022
Open
ron-a20 mentioned this pull request Sep 13, 2022
Open
This was referenced Sep 13, 2022
Open
Open
wedataintelligence mentioned this pull request Sep 13, 2022
Open
snyk-bot mentioned this pull request Sep 13, 2022
Open
molllyn1 mentioned this pull request Sep 14, 2022
Open
137717unity mentioned this pull request Sep 14, 2022
Open
snyk-bot mentioned this pull request Sep 25, 2022
Open
wedataintelligence mentioned this pull request Sep 26, 2022
Open
137717unity mentioned this pull request Sep 26, 2022
Open
snyk-bot mentioned this pull request Sep 27, 2022
Open
wedataintelligence mentioned this pull request Sep 29, 2022
Open
This was referenced Sep 29, 2022
Open
Open
This was referenced Oct 5, 2022
Open
Open
snyk-bot mentioned this pull request Oct 6, 2022
Open
137717unity mentioned this pull request Oct 7, 2022
Open
wedataintelligence mentioned this pull request Oct 7, 2022
Open
This was referenced Oct 8, 2022
Open
Open
137717unity mentioned this pull request Oct 9, 2022
Open
snyk-bot mentioned this pull request Oct 9, 2022
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

richardlau richardlau left review comments
jasnell jasnell approved these changes
Trott Trott approved these changes
benjamingr benjamingr approved these changes
gireeshpunathil gireeshpunathil approved these changes
RaisinTen RaisinTen approved these changes
+1 more reviewer
fhinkel fhinkel approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

Back | FazBrowse Home | New Git URL