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 linting, formatting #2

Merged
merged 20 commits into from
Dec 22, 2023
Merged

Add linting, formatting #2

merged 20 commits into from
Dec 22, 2023

Conversation

tanaya-mankad
Copy link
Contributor

This branch contains CI actions for checking pylint output and mypy output.

  • Two mypy checks are excluded for clarity - basically don't check third-party imports, and don't check functions with no annotations.
  • Failure of either of these checks should not stop the following CI actions

An example of ReStructured Text (sphinx) comment style is also included in a source file.

@tanaya-mankad tanaya-mankad added the enhancement New feature or request label Dec 5, 2023
@tanaya-mankad tanaya-mankad self-assigned this Dec 5, 2023
.pylintrc Outdated
Comment on lines 584 to 585
overgeneral-exceptions=BaseException,
Exception
Copy link
Member

Choose a reason for hiding this comment

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

pylint: Command line or configuration file:1: UserWarning: 'BaseException' is not a proper value for the 'overgeneral-exceptions' option. Use fully qualified name (maybe 'builtins.BaseException' ?) instead. This will cease to be checked at runtime when the configuration upgrader is released.
pylint: Command line or configuration file:1: UserWarning: 'Exception' is not a proper value for the 'overgeneral-exceptions' option. Use fully qualified name (maybe 'builtins.Exception' ?) instead. This will cease to be checked at runtime when the configuration upgrader is released.

continue-on-error: true
- name: Static type checking
run: |
poetry run mypy atheneum #'${{github.workspace}}
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to run this on our tests, too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking not, actually. If we're writing tests that are typing-complicated, we have bigger problems. But I guess I don't mind either way.

def __init__(self) -> None:
pass

def answer(self, exclamation_str):
Copy link
Member

Choose a reason for hiding this comment

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

Hungarian notation???

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sarcasm is unbecoming.

Comment on lines 10 to 14
"""
Return the answer to Life, the Universe, and Everything.

:param exclamation_str: Print how you feel about the answer
"""
Copy link
Member

Choose a reason for hiding this comment

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

At some point we'll want to standardize how we do API documentation. I assume this is a standard syntax for something like sphynx?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this is ReStructured Text that sphinx uses. Tthis looks like a pointless function signature, but I wanted to contrive an example to use the RST comments...

@nealkruis nealkruis merged commit 4d8eded into main Dec 22, 2023
15 checks passed
@nealkruis nealkruis deleted the add-linting-formatting branch December 22, 2023 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants