Skip to content

Remove excessive copyright/license boilerplate - #311

Closed
isaacs wants to merge 1 commit into
nodejs:v1.xfrom
isaacs:delic
Closed

isaacs wants to merge 1 commit into
nodejs:v1.xfrom
isaacs:delic

Conversation

@isaacs

@isaacs isaacs commented Jan 12, 2015

Copy link
Copy Markdown
Contributor

The copyright and license notice is already in the LICENSE file. There
is no justifiable reason to also require that it be included in every
file, since the individual files are not individually distributed except
as part of the entire package.

Since 1,105 files are changed in this diff, reviewers may need to pull it and check manually. I went through the list myself, and only lib/_tls_wrap.js had any unexpected issues, because of a stray comment in the middle of the copyright block that appeared to be the result of a past merge conflict oversight. (As if we needed more evidence that these copyright blocks are a troublesome eyesore!)

@isaacs isaacs mentioned this pull request Jan 12, 2015
@cole-gillespie

Copy link
Copy Markdown

+1

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.

Wut? Is it relevant?

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.

Oh, indeed, that is weird. I'll back that out.

@indutny

indutny commented Jan 12, 2015

Copy link
Copy Markdown
Member

I guess, the sed script gone wilde? ;) Other than minor issue - LGTM

@bnoordhuis

Copy link
Copy Markdown
Member

The changes to deps/npm will be undone the next time it's upgraded, I think?

@isaacs

isaacs commented Jan 12, 2015

Copy link
Copy Markdown
Contributor Author

Backed out changes to deps. That was a mistake. We'll have to do a similar de-boilerplate change in other places, maybe.

@isaacs

isaacs commented Jan 12, 2015

Copy link
Copy Markdown
Contributor Author

Script:

#!/usr/bin/env bash

files=($(git grep -l '// Copyright Joyent, Inc. and other Node contributors.'))

for f in "${files[@]}"; do
  echo $f
  tail -n +21 "$f" > "$f".bak
  mv "$f".bak "$f"
  while [ "$(head -n1 "$f")" = "" ] && [ "$(wc -l "$f" | awk '{print $1}')" -gt 0 ]; do
    tail -n +2 "$f" > "$f".bak
    mv "$f".bak "$f"
  done
done

@piscisaureus

Copy link
Copy Markdown
Contributor

grep "copyright" -i -r * | grep -i "ben noordhuis"

@bnoordhuis

Copy link
Copy Markdown
Member

I think this PR already needs a rebase. Things move fast around here.

The copyright and license notice is already in the LICENSE file.  There
is no justifiable reason to also require that it be included in every
file, since the individual files are not individually distributed except
as part of the entire package.
@isaacs

isaacs commented Jan 12, 2015

Copy link
Copy Markdown
Contributor Author

Rebased on v1.x

On Mon, Jan 12, 2015 at 2:05 PM, Ben Noordhuis notifications@github.com
wrote:

I think this PR already needs a rebase. Things move fast around here.

—
Reply to this email directly or view it on GitHub
#311 (comment).

@isaacs

isaacs commented Jan 12, 2015

Copy link
Copy Markdown
Contributor Author

@piscisaureus The stuff in deps, and anything not covered by the LICENSE file, should still have its obnoxious boilerplate. If Ben wants to remove his copyright notices, I'll leave that to @bnoordhuis.

@bnoordhuis

Copy link
Copy Markdown
Member

The only things that are (c) me are src/queue.h and a test from my punycode library. The former is a verbatim copy from libuv, the latter can be stripped for all I care.

Apropos this PR, rubber-stamp LGTM.

@bnoordhuis

Copy link
Copy Markdown
Member

@isaacs I see you landed this but you forgot the PR-URL and Reviewed-By tags.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants