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-docstrings #52

Merged
merged 8 commits into from
Aug 1, 2022
Merged

Add flake8-docstrings #52

merged 8 commits into from
Aug 1, 2022

Conversation

dalonsoa
Copy link
Collaborator

Description

Adds flake8-docstrings plugin to check the docstrings in the code while running the linter.

Fixes #36

Type of change

  • New feature (non-breaking change which adds functionality)
  • Optimization (back-end change that speeds up the code)
  • Bug fix (non-breaking change which fixes an issue)

Key checklist

  • Make sure you've run the pre-commit checks: $ pre-commit run -a
  • All tests pass: $ poetry run pytest

Further checks

N/A

  • Code is commented, particularly in hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

@dalonsoa dalonsoa mentioned this pull request Jul 29, 2022
7 tasks
@LivDaniel
Copy link
Collaborator

I'm not sure I have enough knowledge to be a competent reviewer on this. My only thought was did David say we should exclude Fiona for now or did we decide to keep it in?

@davidorme
Copy link
Collaborator

I think we should also add this to pre-commit as well:

  - repo: https://gitlab.com/pycqa/flake8
    rev: 3.9.2
    hooks:
      - id: flake8
        additional_dependencies: [flake8-docstrings]

@dalonsoa
Copy link
Collaborator Author

Done!

@davidorme
Copy link
Collaborator

Ack. Should have expected that the docstrings would fail. I did some fixing in the PR for #51 - not sure if I hit all of them.

@dalonsoa dalonsoa closed this Jul 29, 2022
@dalonsoa dalonsoa reopened this Jul 29, 2022
@dalonsoa
Copy link
Collaborator Author

I'll try to sort this out on Monday

@davidorme
Copy link
Collaborator

It's failing on my messy docstrings 😞 I've merged in from develop and cleaned up the remaining ones.

@davidorme davidorme requested a review from alexdewar July 29, 2022 16:38
@davidorme davidorme merged commit 7b03b3f into develop Aug 1, 2022
@davidorme davidorme deleted the ci/add-docstrings-checks branch August 1, 2022 09:45
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.

Check docstrings as part of pre-commit, CI etc.
3 participants