Skip to content

doc: add note in BUILDING.md about running make distclean - #31542

Closed
swagatata wants to merge 3 commits into
nodejs:masterfrom
swagatata:swagat-branch
Closed

doc: add note in BUILDING.md about running make distclean#31542
swagatata wants to merge 3 commits into
nodejs:masterfrom
swagatata:swagat-branch

Conversation

@swagatata

@swagatataswagatata commented Jan 28, 2020•
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes: #28675

Checklist

@nodejs-github-botnodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. labels Jan 28, 2020
mscdex
mscdex reviewed Jan 28, 2020
View reviewed changes
Comment threadBUILDING.md Outdated
@mscdex

mscdex commented Jan 28, 2020

Copy link
Copy Markdown
Contributor

May want to also mention the need to re-run the make command after make distclean.

@swagatata
swagatataforce-pushed the swagat-branch branch 2 times, most recently from 56e23fa to 2ca77d7CompareJanuary 29, 2020 00:50
mscdex
mscdex reviewed Jan 29, 2020
View reviewed changes
Comment threadBUILDING.md Outdated
Trott
Trott reviewed Jan 29, 2020
View reviewed changes
Comment threadBUILDING.md Outdated
@Trott

Trott commented Jan 29, 2020

Copy link
Copy Markdown
Member

Welcome @swagatata and thanks for the pull request. Small documentation pull requests can attract a lot of comments, so your patience is appreciated!

mhdawson
mhdawson approved these changes Jan 30, 2020
View reviewed changes

@mhdawsonmhdawson 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.

LGTM after suggestions.

@Trott

This comment has been minimized.

Sign in to view
@Trott

Trott commented Feb 1, 2020

Copy link
Copy Markdown
Member

I went ahead and applied the earlier nits via --fixup.. So, revising, here are the remaining suggestions:

  • I think this suggestion is in the wrong place. It appears in the build flow description so it looks like it's part of the happy path. It should be in a troubleshooting section instead, or at least at the very, very end of the section.

  • It should explain to people what make distclean does.

  • It should warn the user that after running make distclean, they will have to run ./configure again if they had any non-default options because make distclean removes config.mk (unlike make clean).

  • It should warn the user that after running make distclean, they will need to do a full build so it will take a long time.

  • This should probably document make clean as well, along with the difference between clean and distclean.

@swagatata
swagatataforce-pushed the swagat-branch branch 2 times, most recently from 984ed3d to 97741b7CompareFebruary 1, 2020 23:52
@swagatata

swagatata commented Feb 1, 2020

Copy link
Copy Markdown
ContributorAuthor

I have tried to follow @Trott 's suggestions, adding a "cleaning up the build" section and "troubleshooting" section.

I'm not very well versed with what exactly "make clean" does and how it differs from "make distclean" so I'd welcome some notes to add there.

Trott
Trott requested changes Feb 2, 2020
View reviewed changes

@TrottTrott 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.

Needs a little bit more editing. Just leaving this here so no one lands this without those edits.

Trott
Trott approved these changes Feb 2, 2020
View reviewed changes

@TrottTrott 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.

I moved stuff around, changed headers, but the basic gist and fundamental wording is all still there, I think. PTAL.

@swagatata

swagatata commented Feb 7, 2020

Copy link
Copy Markdown
ContributorAuthor

Who can merge this?

@Trott

Trott commented Feb 7, 2020

Copy link
Copy Markdown
Member

Landed in a733c18

@TrottTrott closed this Feb 7, 2020
Trott pushed a commit that referenced this pull request Feb 7, 2020
@swagatata@Trott
doc: add note in BUILDING.md about running make distclean
a733c18
Fixes: #28675
PR-URL: #31542
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@swagatata
swagatata deleted the swagat-branch branch February 15, 2020 18:36
codebytere pushed a commit that referenced this pull request Feb 17, 2020
@swagatata@codebytere
doc: add note in BUILDING.md about running make distclean
b61b85c
Fixes: #28675
PR-URL: #31542
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@codebyterecodebytere mentioned this pull request Feb 17, 2020
Merged
codebytere pushed a commit that referenced this pull request Mar 15, 2020
@swagatata@codebytere
doc: add note in BUILDING.md about running make distclean
eee0777
Fixes: #28675
PR-URL: #31542
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
codebytere pushed a commit that referenced this pull request Mar 17, 2020
@swagatata@codebytere
doc: add note in BUILDING.md about running make distclean
b5b0dc9
Fixes: #28675
PR-URL: #31542
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@codebyterecodebytere mentioned this pull request Mar 17, 2020
Merged
codebytere pushed a commit that referenced this pull request Mar 30, 2020
@swagatata@codebytere
doc: add note in BUILDING.md about running make distclean
e83af20
Fixes: #28675
PR-URL: #31542
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes
@TrottTrottTrott approved these changes
+2 more reviewers
@mscdexmscdexmscdex left review comments
@mhdawsonmhdawsonmhdawson approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

buildIssues and PRs related to build files or the CI.docIssues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

add note in BUILDING.md about running make distclean

6 participants

@swagatata@mscdex@Trott@jasnell@mhdawson@nodejs-github-bot