Add pre-commit hook to verify required apache headers - #91
Conversation
…e repo for existing apache header
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #91 +/- ##
=========================================
Coverage ? 93.84%
=========================================
Files ? 9
Lines ? 1138
Branches ? 0
=========================================
Hits ? 1068
Misses ? 70
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
JoshSEdwards
left a comment
JoshSEdwards
left a comment
There was a problem hiding this comment.
Thanks for adding this, and I'm glad it did catch a few files that were missing headers! Just a few minor comments below.
I think it would be helpful to document how to install and enable the hook in the contributor setup docs. Could we also add pre-commit run --all-files to CI, after
installing pre-commit there? That would make the local workflow clear and ensure the check still runs when someone hasn't enabled the local hook.
| for token in tokenize.generate_tokens(file.readline): | ||
| # Check if the current token is a comment | ||
| if token.type == tokenize.COMMENT: | ||
| # Check if apache link string is inside that comment |
There was a problem hiding this comment.
This looks pretty good, but it currently allows for the Apache link string to be in a comment anywhere in the file. For example, a Python file with no header but a trailing comment such as # http://www.apache.org/licenses/LICENSE-2.0 passes. I think we should restrict the check to the initial header region, allowing only a shebang/encoding declaration before it, or if it's easier we can just validate the full expected header.
| @@ -0,0 +1,11 @@ | |||
|
|
|||
There was a problem hiding this comment.
This is nitpicky, but can we remove the blank lines at the beginning and end of this file?
4 participants
Closes issue #7
Problem: Needed a way to automatically check that the apache license header exists in all relevant files to ensure it does not accidentally get left out from future commits to any relevant source files.
Approach: I implemented a pre-commit hook (.pre-commit-config.yaml) that calls a small python script (scripts/apache_header_check.py). The script checks all ‘.py’ files in the repo to see if the license boiler text exists in the comments of each file. If it does not, it fails and lets you know which file is missing it. I did not check the full copyright text as I noticed the copyright year isn’t always 2018 , so I only checked ‘http://www.apache.org/licenses/LICENSE-2.0', but can rework to include full copyright text check if needed.
Validation: You can test this by choosing a current ‘.py’ file that has the boiler plate text and removing it. When you go to commit the file, it will error out and let you know that the file is missing the boiler plate text. If you re-add the license information and re-commit it will pass. You can also manually run the command in the terminal with: pre-commit run apache-license-header-check --all-files