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

Address PR comments for 787 ljw pre commit hook compliant #794

Conversation

tomvothecoder
Copy link
Collaborator

@tomvothecoder tomvothecoder commented Nov 4, 2021

This PR fixes the GH Actions build issues in PR #787.

Summary of Changes

Address QA Tool Issues

  • Add test_placeholder.py to workaround pytest error code 5 (no tests ran)
    • Remove this file when adding real tests
  • Add pre_commit to dev.yml
  • Workaround flake8 errors related to F821 undefined name
    • Need to address FIXME: placeholder comments in the future

Configuration Updates

  • Remove mypy from .pre-commit-config.yaml
  • Remove "Upload Coverage Report" step from "build" job in GH Actions workflow
    • Only needed if you want Codecov setup to host repo code coverage reports

- Add `pre_commit` to `dev.yml`
- Update `mypy` `python_version=3.9` in `setup.cfg`
- Remove `mypy` from `.pre-commit-config.yaml`
- Workaround flake8 errors related to F821 undefined name
@lee1043
Copy link
Contributor

lee1043 commented Nov 4, 2021

Removing mypy sounds reasonable at this moment because most of PMP code has been wrote with dynamic typing, instead of static typing as checked by mypy.

@lee1043
Copy link
Contributor

lee1043 commented Nov 4, 2021

@tomvothecoder thanks for adding placeholder for testing. I believe this corresponds to the task 5 of the note.

@lee1043 lee1043 merged commit 3ed0eb0 into PCMDI:787_ljw_pre-commit-hook-compliant Nov 4, 2021
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