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

Add flake8 checks for license and copyright headers #4657

Merged
merged 7 commits into from May 6, 2019

Conversation

m4dcoder
Copy link
Contributor

@m4dcoder m4dcoder commented May 2, 2019

Add custom flake8 extension to check that the Apache 2.0 license is included in code files.
Add Apache 2.0 license header to files that does not have it. Fix Apache 2.0 license header if it
doesn't match the header that is expected.

@m4dcoder m4dcoder added the WIP label May 2, 2019
@m4dcoder m4dcoder requested review from Kami and bigmstone May 2, 2019 02:41
@m4dcoder
Copy link
Contributor Author

m4dcoder commented May 2, 2019

@Kami @bigmstone I'm going to fix this first and then add an extension to check copyright header.

@m4dcoder m4dcoder force-pushed the flake8-license-copyrights branch from 6d48729 to 57cefab Compare May 2, 2019 04:26
@m4dcoder m4dcoder changed the title Add flake8 extension to check license header Add flake8 check for license and copyright headers May 2, 2019
@m4dcoder m4dcoder changed the title Add flake8 check for license and copyright headers Add flake8 checks for license and copyright headers May 2, 2019
@m4dcoder m4dcoder force-pushed the flake8-license-copyrights branch 2 times, most recently from ba8f033 to 18c73ad Compare May 2, 2019 04:35
Add the current license header to files that do not have the header. Fix the license header in files if the text does not match.
Update the test_content_version submodule to pull the latest commit with the Apache 2.0 license header added to files.
Replace the custom Apache 2.0 license header to the standard Apache 2.0 license header in code files.
Add the hacking module to check code files for the standard Apache 2.0 license header. Update the flake8 config file and disable all other hacking checks.
Add copyright header above license header on all code files.
Add flake8-copyright to test requirements and configure the copyright check in the flake8 configuration file.
@m4dcoder m4dcoder force-pushed the flake8-license-copyrights branch from 18c73ad to 9b8d918 Compare May 5, 2019 05:53
@m4dcoder m4dcoder removed the WIP label May 5, 2019
@Kami Kami added this to the 3.1.0 milestone May 5, 2019
@@ -436,7 +436,7 @@ requirements: virtualenv .sdist-requirements install-runners
(cd ${ROOT_DIR}/st2common; ${ROOT_DIR}/$(VIRTUALENV_DIR)/bin/python setup.py develop --no-deps)

# Some of the tests rely on submodule so we need to make sure submodules are check out
git submodule update --init --recursive
git submodule update --recursive --remote
Copy link
Member

Choose a reason for hiding this comment

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

Just curious why this change is needed.

@@ -2,6 +2,7 @@
coverage==4.5.2
pep8==1.7.1
flake8==3.7.7
hacking
Copy link
Member

Choose a reason for hiding this comment

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

Probably not a bad idea to pin it to a specific version to avoid surprises when a new version if published / similar. This has bitten us many times already in the past, same for flake8-copyright.

Although checking hacking docs (https://pypi.org/project/hacking/) it looks like hacking itself installs specific versions of pep8, etc. so there could be potential conflicts down the road.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Kami Pinned both flake8-copyright and hacking in test-requirements.txt. Why did we pin a specific version of pep8 (pep8==1.7.1)?

Copy link
Member

@Kami Kami May 6, 2019

Choose a reason for hiding this comment

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

New versions (> 2.0.0, IIRC), only work with Python 3.

EDIT: Nvm, that's pylint and not pep8.

@Kami Kami added the license label May 5, 2019
Pin version for flake8-copyright and hacking to avoid unexpected conflict due to version changes.
@m4dcoder m4dcoder merged commit 9edc3fb into master May 6, 2019
@m4dcoder m4dcoder deleted the flake8-license-copyrights branch May 6, 2019 16:36
Kami added a commit to StackStorm/lint-configs that referenced this pull request May 7, 2019
Kami added a commit that referenced this pull request May 7, 2019
72d3b7ca9 Merge pull request #1 from StackStorm/update_lint_configs
2709c47a2 Update lint config with changes from #4657.

git-subtree-dir: lint-configs
git-subtree-split: 72d3b7ca9e2d775b58780e49580d11c3c6414d27
@Kami Kami mentioned this pull request May 7, 2019
Kami added a commit to StackStorm/st2-rbac-backend that referenced this pull request May 7, 2019
73fe88f Fix merge conflict.
72d3b7c Merge pull request #1 from StackStorm/update_lint_configs
2709c47 Update lint config with changes from StackStorm/st2#4657.

git-subtree-dir: lint-configs
git-subtree-split: 73fe88f2da7403d69bc5ebd7c6d22d12860c3744
Kami added a commit to StackStorm/st2-auth-ldap that referenced this pull request May 7, 2019
73fe88f Fix merge conflict.
72d3b7c Merge pull request #1 from StackStorm/update_lint_configs
2709c47 Update lint config with changes from StackStorm/st2#4657.
418d2eb Sync flake8 config with StackStorm/st2 master.
9b6b10b Update flake8 config so we ignore build/ and dist/ directories.
e43b03a Update pylint config.

git-subtree-dir: lint-configs
git-subtree-split: 73fe88f2da7403d69bc5ebd7c6d22d12860c3744
m4dcoder added a commit that referenced this pull request May 10, 2019
f0fd0b6 Merge pull request #2 from StackStorm/python-license-check
63e6d42 Add new .flake-proprietary config file which is to be used with proprietary repos.
67d2846 Update flake8 config to enable extensions
a164dc1 Add .gitignore to project
13e0509 Add flake8 plugin to check license header
302b88c Also ignore H210 errors.
ffbe682 Enable H checks to missing license errors are reported.
3be3554 Merge branch 'update_lint_configs'
9a5b6db Add a docstring.
73fe88f Fix merge conflict.
72d3b7c Merge pull request #1 from StackStorm/update_lint_configs
2709c47 Update lint config with changes from #4657.

git-subtree-dir: lint-configs
git-subtree-split: f0fd0b61e29a9623eeb87aaf82c3a98aab9271ff
@m4dcoder m4dcoder self-assigned this Jun 22, 2019
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.

None yet

2 participants