Add linting pre-commit hook and hook install script #86

Open
j24chung wants to merge 1 commits from feature-80 into master
Collaborator
There is no content yet.
j24chung added 1 commit 3 months ago
da51c4309c
Add linting pre-commit hook and hook install script
r345liu requested changes 2 months ago
r345liu left a comment
Owner

Might want to consider the case where file name can be white space. Just adding a IFS='\n' before the for loop is probably enough.

Might want to consider the case where file name can be white space. Just adding a `IFS='\n'` before the for loop is probably enough.
merenber requested changes 2 months ago
PASS=true
# Check for flake8
Owner

I suggest using the flake8 script which gets installed in venv/bin, so that the user doesn't have to install a ilbrary globally if they don't want to.

I suggest using the flake8 script which gets installed in venv/bin, so that the user doesn't have to install a ilbrary globally if they don't want to.
Owner

I think we can assume git commit would be running inside virtual env, plus venv directory can be located elsewhere too

I think we can assume git commit would be running inside virtual env, plus venv directory can be located elsewhere too
for FILE in $STAGED_FILES
do
flake8 "$FILE"
Owner

Is there a reason why we need to invoke flake8 on each file separately? If you just run flake8 once from the root directory, it'll lint all of the source files at once.

Is there a reason why we need to invoke flake8 on each file separately? If you just run `flake8` once from the root directory, it'll lint all of the source files at once.
flake8 "$FILE"
if [[ "$?" == 0 ]]; then
echo -e "\t\033[32mPassed: $FILE\033[0m"
Owner

flake8 provides its own diagnostic messages for each lint error, so I don't think we need to add an extra "Passed" or "Failed" message.

flake8 provides its own diagnostic messages for each lint error, so I don't think we need to add an extra "Passed" or "Failed" message.
echo -e "\nPython linting complete!\n"
if ! $PASS; then
echo -e "\033[41mCOMMIT FAILED:\033[0m Your commit contains files that should pass ESLint but do not. Please fix the ESLint errors and try again.\n"
Owner

Maybe we should change "ESLint" to "flake8"?

Maybe we should change "ESLint" to "flake8"?

Reviewers

r345liu requested changes 2 months ago
merenber requested changes 2 months ago
All checks were successful
continuous-integration/drone/pr Build is passing
Required
Details
This Pull Request doesn't have enough approvals yet. 0 of 1 approvals granted.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
Sign in to join this conversation.
No Label
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: public/pyceo#86
Loading…
There is no content yet.