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

NAS-129755 / 25.04 / Introduce docstrings and type annotations #7

Merged
merged 6 commits into from
Aug 30, 2024

Conversation

creatorcary
Copy link
Contributor

@creatorcary creatorcary commented Aug 5, 2024

The main features of this PR are:

  • verbose type specification for what can be received from the server in jsonrpc.py using TypedDict
  • Google-style docstrings
  • a README
  • pytype for CI type checking to enforce type safety

Minor syntactical changes were made in some places to appease pytype and other type checkers, esp. pyright.

As a follow-up to this PR, I recommend we import the type specification defined in jsonrpc.py in middleware and use it to enforce type safety on the server-side as well.

Note: I have skipped adding type hints/docstrings in legacy.py.

@bugclerk bugclerk changed the title Introduce docstrings and type annotations NAS-129755 / None / Introduce docstrings and type annotations Aug 5, 2024
@bugclerk
Copy link

bugclerk commented Aug 5, 2024

@creatorcary creatorcary added the documentation Improvements or additions to documentation label Aug 5, 2024
@yocalebo yocalebo changed the title NAS-129755 / None / Introduce docstrings and type annotations NAS-129755 / 25.04 / Introduce docstrings and type annotations Aug 12, 2024
Base automatically changed from jsonrpc to master August 15, 2024 16:00
@creatorcary creatorcary marked this pull request as ready for review August 15, 2024 16:55
@creatorcary creatorcary requested review from themylogin and a team August 15, 2024 16:55
README.md Show resolved Hide resolved
truenas_api_client/utils.py Outdated Show resolved Hide resolved
truenas_api_client/__init__.py Outdated Show resolved Hide resolved
Copy link
Contributor

@themylogin themylogin left a comment

Choose a reason for hiding this comment

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

LGTM!

@creatorcary creatorcary merged commit 89b9b3c into master Aug 30, 2024
1 check passed
@creatorcary creatorcary deleted the NAS-129755 branch August 30, 2024 12:57
@bugclerk
Copy link

JIRA ticket https://ixsystems.atlassian.net/browse/NAS-129755 is targeted to the following versions which have not received their corresponding PRs: 25.04

@bugclerk
Copy link

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.

@truenas truenas locked as resolved and limited conversation to collaborators Aug 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants