Skip to content
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.

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

chore: Check copyright and license as one joined substring #8815

Merged
merged 3 commits into from Aug 13, 2019

Conversation

@pedrottimark
Copy link
Collaborator

pedrottimark commented Aug 12, 2019

Summary

What do y’all think? In the first draft of #8783 a joined substring found some mistakes:

  • 6 files with extra redundant All rights reserved. line between copyright and license
  • missing period at end of copyright line in checkCopyrightHeaders.js itself ;)

that searching for the 3 lines independently (in the merged code) does not find

Test plan

yarn check-copyright-headers

After #8809 the script will pass when run locally on Windows system, correct?

Tim, thanks again for researching such a smart solution ❤️

' *',
' * This source code is licensed under the MIT license found in the',
' * LICENSE file in the root directory of this source tree.',
].join('\n');

function needsCopyrightHeader(file) {
const contents = getFileContents(file);

// Match lines individually to avoid false positive for:

This comment has been minimized.

Copy link
@jeysal

jeysal Aug 13, 2019

Collaborator

@pedrottimark Yes the line endings now shouldn't be a problem as they're required to be LF. But this comment says another reason why lines are checked individually is the ability to use line comments instead of a block comment. Looks like this possibility is never used (CI is green) but wondering if these line substrings are the standard license header check for FB or something.

This comment has been minimized.

Copy link
@pedrottimark

pedrottimark Aug 13, 2019

Author Collaborator

Sorry to make you do the proof reading that I should have done, because I wrote that comment in the previous PR and that sentence was a step in the wrong direction.

@SimenB
SimenB approved these changes Aug 13, 2019
@jeysal jeysal merged commit 0d48344 into facebook:master Aug 13, 2019
10 of 11 checks passed
10 of 11 checks passed
ci/circleci: test-jest-circus Your tests failed on CircleCI
Details
ci/circleci: lint-and-typecheck Your tests passed on CircleCI!
Details
ci/circleci: test-browser Your tests passed on CircleCI!
Details
ci/circleci: test-node-10 Your tests passed on CircleCI!
Details
ci/circleci: test-node-11 Your tests passed on CircleCI!
Details
ci/circleci: test-node-6 Your tests passed on CircleCI!
Details
ci/circleci: test-node-8 Your tests passed on CircleCI!
Details
ci/circleci: test-or-deploy-website Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
facebook.jest #20190813.1 succeeded
Details
@pedrottimark pedrottimark deleted the pedrottimark:copyright-license branch Aug 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.