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

Typing charm.py #791

Merged
merged 11 commits into from
Aug 2, 2022
Merged

Typing charm.py #791

merged 11 commits into from
Aug 2, 2022

Conversation

PietroPasotti
Copy link
Contributor

@PietroPasotti PietroPasotti commented Jul 4, 2022

Typed ops/charm.py

Minor changes to framework.py and vendor/yaml.py types as well.

Checklist

  • Have any types changed? If so, have the type annotations been updated?
  • [n/a] If this code exposes model data, does it do so thoughtfully, in a way that aids understanding?
  • [n/a] Do error messages, if any, present charm authors or operators with enough information to solve the problem?

QA steps

tox -e static

Changelog

  • Statically typed charm.py

@rwcarlsen
Copy link
Contributor

This looks like it maybe was built on top of the relation keys typing PR. Could you rebase it so this only has the charm.py typing commits?

@PietroPasotti
Copy link
Contributor Author

This looks like it maybe was built on top of the relation keys typing PR. Could you rebase it so this only has the charm.py typing commits?

Done!

Copy link
Contributor

@rwcarlsen rwcarlsen left a comment

Choose a reason for hiding this comment

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

Pretty good, just a few relatively minor things.

ops/charm.py Outdated Show resolved Hide resolved
ops/charm.py Show resolved Hide resolved
ops/charm.py Outdated Show resolved Hide resolved
ops/charm.py Outdated Show resolved Hide resolved
ops/charm.py Outdated Show resolved Hide resolved
ops/charm.py Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
@jnsgruk jnsgruk merged commit 93ecb33 into canonical:main Aug 2, 2022
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.

3 participants