8000 pre commit hook to verify file sizes by seal6363 · Pull Request #2151 · allenai/allennlp · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Dec 16, 2022. It is now read-only.

pre commit hook to verify file sizes #2151

Merged
merged 13 commits into from
Dec 12, 2018
Merged

pre commit hook to verify file sizes #2151

merged 13 commits into from
Dec 12, 2018

Conversation

seal6363
Copy link
Contributor
@seal6363 seal6363 commented Dec 6, 2018 •
edited
Loading

Fixes #1753

@seal6363
Copy link
Contributor Author
seal6363 commented Dec 6, 2018

./scripts/install-hooks.bash

@schmmd
Copy link
Member
schmmd commented Dec 6, 2018

@seal6363 can you add this to scripts/verify.py as a check? Next we can set it up to run in TeamCity as well.

The commit hooks are great--but they rely on the user actually installing them which--in my experience--often doesn't happen.

Nit: I usually use the .sh suffix for bash scripts.

schmmd
schmmd reviewed Dec 7, 2018
View reviewed changes
scripts/verify.py Outdated
except CalledProcessError:
# squelch the exception stacktrace
sys.exit(1)

if __name__ == "__main__":
checks = ['pytest', 'pylint', 'mypy', 'build-docs', 'check-docs', 'check-links', 'check-requirements']
checks = ['pytest', 'pylint', 'mypy', 'build-docs', 'check-docs', 'check-links', 'check-requirements', 'check-files-sizes']
Copy link
Member

Choose a reason for hiding this comment

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

Would you rename this "check-large-files"?

schmmd
schmmd reviewed Dec 7, 2018
View reviewed changes
@schmmd
Copy link
Member
schmmd commented Dec 7, 2018

@seal6363 before merging please try out ./scripts/verify.py both with a passing configuration and with a large file (where it should fail).

@seal6363
Copy link
Contributor Author
seal6363 commented Dec 8, 2018

Don't merge. it's not working.

@seal6363 seal6363 merged commit da761e8 into allenai:master Dec 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@schmmd schmmd schmmd approved these changes

Assignees

@schmmd schmmd

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@seal6363 @schmmd @matt-gardner
0