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

Update flake8 and mypy stuff #781

Merged
merged 3 commits into from
Jan 22, 2022

Conversation

jakobandersen
Copy link
Collaborator

  • Remove #noqa comments
  • Switch comment-based type annotation to variable type hints
  • Make mypy warn about redundant things
  • General mypy fixes

There is still one error left:

breathe/renderer/sphinxrenderer.py:439: error: List item 0 has incompatible type "Tuple[str]"; expected "Tuple[str, str]"

"inner": flag,
}
option_spec = _DoxygenContentBlockDirective.option_spec.copy()
option_spec.update({"inner": flag})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Out of interest, why do you prefer this formulation? Does it have an objective benefit or just preferred style?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is just a style choice, to be consistent with the equivalent cases I have seen in Sphinx to more quickly spot what is going on.

Copy link
Collaborator

@michaeljones michaeljones left a comment

Choose a reason for hiding this comment

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

Looks like a great improvement. Much respect for grinding through it.

@michaeljones
Copy link
Collaborator

@jakobandersen - please merge when you're happy with it :)

- Remove noqa comments
- Switch comment-based type annotation to variable type hints
- Make mypy warn about redundant things
- General mypy fixes
@jakobandersen jakobandersen merged commit 4ae3da2 into breathe-doc:master Jan 22, 2022
@jakobandersen jakobandersen deleted the lint_update branch January 22, 2022 12:08
@jakobandersen
Copy link
Collaborator Author

Thanks, merged.

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.

2 participants