Skip to content

scripts and tools: Update copyright_header.py script #16314

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Jul 2, 2019
Merged

scripts and tools: Update copyright_header.py script #16314

merged 3 commits into from
Jul 2, 2019

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Jun 29, 2019 •
edited
Loading

Now the copyright_header.py script handles Objective-C source files *.mm:

src/qt/macdockiconhandler.mm
src/qt/macnotificationhandler.mm
src/qt/macos_appnap.mm

Also the only occurrence of Bitcoin Core Developers replaced with ubiquitous The Bitcoin Core developers.

EDITED:
The reason to remove "Sam Rushing" is (on master):

$ git grep "Sam Rushing"
contrib/devtools/copyright_header.py:    "Sam Rushing\n",

hebasto added 3 commits June 29, 2019 18:35
@hebasto
Include Objective-C source files
01fafe5 
The copyright_header.py script will process Objective-C source files 
(*.mm) as other ones.
@hebasto
Drop no-longer-relevant copyright holder name
621463d
@hebasto
Fix: "Bitcoin Core" -> "The Bitcoin Core"
ca11606
@DrahtBot
Copy link
Contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #15257 (Scripts and tools: Bump flake8 to 3.7.7 by Empact)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

promag
promag reviewed Jun 30, 2019
View reviewed changes
contrib/devtools/copyright_header.py
"BitPay Inc\.\n",
"University of Illinois at Urbana-Champaign\.\n",
"Pieter Wuille\n",
"Wladimir J. van der Laan\n",
"Jeff Garzik\n",
"Jan-Klaas Kollhof\n",
"Sam Rushing\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is Sam removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

$ git grep "Sam Rushing"
contrib/devtools/copyright_header.py:    "Sam Rushing\n",
$

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, maybe mention it in the OP.

Copy link
Member Author

Choose a reason for hiding this comment

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

@promag

Ok, maybe mention it in the OP.

Done.

@practicalswift
Copy link
Contributor

practicalswift commented Jun 30, 2019 •
edited
Loading

Concept ACK

Is there any reason to list individual copyright holders that also belong to the group "The Bitcoin Core developers"?

If not it would be nice to reduce the number of individual copyright holders and simplify this script.

If @sipa, @laanwj and @JeremyRubin so allows it would be nice to replace the following with "The Bitcoin Core developers":

$ git grep -iE 'Copyright.*(Pieter Wuille|Wladimir J. van der Laan|Jeremy Rubin)' ":(exclude)src/crypto/ctaes/" ":(exclude)src/leveldb/" ":(exclude)src/secp256k1/" ":(exclude)src/univalue/" ":(exclude)contrib/devtools/"
build_msvc/libsecp256k1_config.h: * Copyright (c) 2013, 2014 Pieter Wuille                             *
contrib/seeds/generate-seeds.py:# Copyright (c) 2014-2017 Wladimir J. van der Laan
src/addrman.cpp:// Copyright (c) 2012 Pieter Wuille
src/addrman.h:// Copyright (c) 2012 Pieter Wuille
src/bech32.cpp:// Copyright (c) 2017 Pieter Wuille
src/bech32.h:// Copyright (c) 2017 Pieter Wuille
src/cuckoocache.h:// Copyright (c) 2016 Jeremy Rubin
src/test/bech32_tests.cpp:// Copyright (c) 2017 Pieter Wuille
test/functional/test_framework/descriptors.py:# Copyright (c) 2019 Pieter Wuille
test/functional/test_framework/key.py:# Copyright (c) 2019 Pieter Wuille
test/functional/test_framework/segwit_addr.py:# Copyright (c) 2017 Pieter Wuille

Perhaps build_msvc/libsecp256k1_config.h being be the exception (since it is part of secp256k1).

@sipa @laanwj @JeremyRubin - would you allow being listed as "The Bitcoin Core developers" instead of as individuals?

@laanwj
Copy link
Member

laanwj commented Jul 1, 2019

code review ACK ca11606

@practicalswift i'm fiine with that but also think simplifying that script is a non-issue

@practicalswift
Copy link
Contributor

@laanwj Do you know the history behind these individual copyright notices? Is there any reason to list individual copyright holders that also belong to the group "The Bitcoin Core developers"?

maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Jul 2, 2019
Merge bitcoin#16314: scripts and tools: Update copyright_header.py sc…
c84349a 
…ript

ca11606 Fix: "Bitcoin Core" -> "The Bitcoin Core" (Hennadii Stepanov)
621463d Drop no-longer-relevant copyright holder name (Hennadii Stepanov)
01fafe5 Include Objective-C source files (Hennadii Stepanov)

Pull request description:

  Now the `copyright_header.py` script handles Objective-C source files `*.mm`:
  ```
  src/qt/macdockiconhandler.mm
  src/qt/macnotificationhandler.mm
  src/qt/macos_appnap.mm
  ```

  Also the only occurrence of `Bitcoin Core Developers` replaced with ubiquitous `The Bitcoin Core developers`.

  EDITED:
  The reason to remove "Sam Rushing" is (on master):
  ```
  $ git grep "Sam Rushing"
  contrib/devtools/copyright_header.py:    "Sam Rushing\n",
  ```

ACKs for top commit:
  laanwj:
    code review ACK ca11606

Tree-SHA512: 446c8fc569f732a6758e765f64110d9faeeffabb69088dd081d7bb730255c87196da96cea51081f4bd49280049fa4ed2ae22091059cb0f89bdc4ef8dd5e63cf0
@maflcko maflcko merged commit ca11606 into bitcoin:master Jul 2, 2019
@hebasto hebasto deleted the 20190629-copyright-headers branch July 2, 2019 15:54
@mmortal03 mmortal03 mentioned this pull request Oct 14, 2019
Closed
kwvg added a commit to kwvg/dash that referenced this pull request Jul 11, 2021
@kwvg
merge bitcoin#16314: Update copyright_header.py script
3ee0971
kwvg added a commit to kwvg/dash that referenced this pull request Jul 11, 2021
@kwvg
merge bitcoin#16314: Update copyright_header.py script
c6d8667
PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Jul 13, 2021
@PastaPastaPasta
Merge pull request #4242 from kittywhiskers/morebps
f1f3236 
merge bitcoin#14785, bitcoin#16314, bitcoin#17052: minor backports
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jul 20, 2021
@hebasto @PiRK
Include Objective-C source files
9e2e162 
Summary:
The copyright_header.py script will process Objective-C source files
(*.mm) as other ones.

Drop no-longer-relevant copyright holder name

This is a backport of [[bitcoin/bitcoin#16314 | core#16314]]

Depends on D9811

Test Plan: `./contrib/devtools/copyright_header.py report .`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D9812
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
2 more reviewers

@promag promag promag left review comments

@practicalswift practicalswift practicalswift left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Scripts and tools
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@hebasto @DrahtBot @practicalswift @laanwj @promag @maflcko