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

build: don't install github templates by jbergstroem · Pull Request #5612 · nodejs/node · GitHub

/ node Public

build: don't install github templates - #5612

Merged
jbergstroem merged 1 commit into
nodejs:masterfrom
jbergstroem:fix/remove-github-templates-installer
Mar 9, 2016
ConversationCommits (1)ChecksFiles changed
Merged

build: don't install github templates#5612
jbergstroem merged 1 commit into
nodejs:masterfrom
jbergstroem:fix/remove-github-templates-installer

Conversation

jbergstroem commented Mar 9, 2016

Copy link
Copy Markdown
Member
  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to [CONTRIBUTING.md][0]?
  • If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?
  • Is a documentation update included (if this change modifies
    existing APIs, or introduces new ones)?

Affected core subsystem(s)

build

Description of change

Avoid putting github templates in the source tarballs.

/cc @nodejs/build

rvagg commented Mar 9, 2016

Copy link
Copy Markdown
Member

lgtm on this individual change.

We also have these in the top level:

.editorconfig
.eslintignore
.eslintrc
.gitattributes
.gitignore
.mailmap
android-configure
AUTHORS
BSDmakefile
CHANGELOG.md
CODE_OF_CONDUCT.md
COLLABORATOR_GUIDE.md
common.gypi
configure
CONTRIBUTING.md
GOVERNANCE.md
LICENSE
Makefile
node.gyp
README.md
ROADMAP.md
vcbuild.bat
WORKING_GROUPS.md

I wouldn't mind getting rid of .editorconfig, .gitattributes, .gitignore, .mailmap, CODE_OF_CONDUCT.md, COLLABORATOR_GUIDE.md, CONTRIBUTING.md, GOVERNANCE.md, ROADMAP.md, WORKING_GROUPS.md. I don't think they add anything valuable to a user downloading a source tarball. @nodejs/build @nodejs/ctc thoughts on why any of these should be kept in the source tarball?

Trott commented Mar 9, 2016

Copy link
Copy Markdown
Member

CODE_OF_CONDUCT.md, COLLABORATOR_GUIDE.md, CONTRIBUTING.md, and GOVERNANCE.md are linked (with relative paths) in the README.md. I'd be inclined to keep them.

Removing GitHub templates from the tarball change: LGTM

mscdex added the build Issues and PRs related to build files or the CI. label Mar 9, 2016

jasnell commented Mar 9, 2016

Copy link
Copy Markdown
Member

+1 to keeping CODE_OF_CONDUCT.md, COLLABORATOR_GUIDE.md, CONTRIBUTING.md and GOVERNANCE.md. The others I'm good with excluding.

jbergstroem commented Mar 9, 2016

Copy link
Copy Markdown
Member Author

@rvagg will open open a discussion that aims to target what a tarball (vs git clone) should contain.

bnoordhuis commented Mar 9, 2016

Copy link
Copy Markdown
Member

Apropos the README, people that download the binaries frequently get confused about the build instructions in it. Perhaps it's better to distribute an abridged version in release tarballs, then we can drop the other *.md files too.

jbergstroem mentioned this pull request Mar 9, 2016
Closed

Trott commented Mar 9, 2016

Copy link
Copy Markdown
Member

Apropos the README, people that download the binaries frequently get confused about the build instructions in it. Perhaps it's better to distribute an abridged version in release tarballs, then we can drop the other *.md files too.

I've certainly thought before that it might be good to either move all build instructions to a separate file or else minimize the instructions in the README and having detailed instructions in a separate file. I'm talking about in the repo, not just tarballs.

build: don't install github templates
7f586c0 
Avoid putting github templates in the source tarballs.

PR-URL: nodejs#5612
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Rich Trott <rtrott@gmail.com>
jbergstroem force-pushed the fix/remove-github-templates-installer branch from 03940ba to 7f586c0 Compare March 9, 2016 23:28
jbergstroem merged commit 7f586c0 into nodejs:master Mar 9, 2016
jbergstroem mentioned this pull request Mar 9, 2016
Merged
4 tasks
evanlucas mentioned this pull request Mar 14, 2016
Closed
evanlucas pushed a commit that referenced this pull request Mar 14, 2016
build: don't install github templates
6071920 
Avoid putting github templates in the source tarballs.

PR-URL: #5612
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Rich Trott <rtrott@gmail.com>
evanlucas mentioned this pull request Mar 14, 2016
Merged
rvagg pushed a commit that referenced this pull request Mar 16, 2016
build: don't install github templates
03b99bf 
Avoid putting github templates in the source tarballs.

PR-URL: #5612
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
build: don't install github templates
4c089a6 
Avoid putting github templates in the source tarballs.

PR-URL: #5612
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
build: don't install github templates
067ce9b 
Avoid putting github templates in the source tarballs.

PR-URL: #5612
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Rich Trott <rtrott@gmail.com>
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

No reviews

Assignees

No one assigned

Labels

build Issues and PRs related to build files or the CI.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL