Skip to content

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

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

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

Conversation

@swagatata
Copy link
Contributor

@swagatata swagatata commented Jan 28, 2020 •
edited
Loading

Fixes: #28675

Checklist

@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. labels Jan 28, 2020
mscdex
mscdex reviewed Jan 28, 2020
View reviewed changes
@mscdex
Copy link
Contributor

mscdex commented Jan 28, 2020

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

@swagatata swagatata force-pushed the swagat-branch branch 2 times, most recently from 56e23fa to 2ca77d7 Compare January 29, 2020 00:50
mscdex
mscdex reviewed Jan 29, 2020
View reviewed changes
Trott
Trott reviewed Jan 29, 2020
View reviewed changes
@Trott
Copy link
Member

Trott commented Jan 29, 2020

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

@mhdawson mhdawson left a comment

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

Trott commented Feb 1, 2020

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 swagatata force-pushed the swagat-branch branch 2 times, most recently from 984ed3d to 97741b7 Compare February 1, 2020 23:52
@swagatata
Copy link
Contributor Author

swagatata commented Feb 1, 2020

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

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

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

@swagatata
Copy link
Contributor Author

swagatata commented Feb 7, 2020

Who can merge this?

@Trott
Copy link
Member

Trott commented Feb 7, 2020

Landed in a733c18

@Trott Trott 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>
@codebytere codebytere 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>
@codebytere codebytere 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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@Trott Trott Trott approved these changes

@mhdawson mhdawson mhdawson approved these changes

+1 more reviewer

@mscdex mscdex mscdex left review comments

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.

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