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

Fully Annotate Library and Mark as PEP-561-compatible #44

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

TeoZosa
Copy link
Contributor

@TeoZosa TeoZosa commented Sep 12, 2021

Background

My dependent logging library recently started compiling modules into C extensions via Mypyc (TeoZosa/structlog-sentry-logger#220). To compile the library successfully, I needed to modify structlog-sentry as part of TeoZosa/structlog-sentry-logger#253.

Changes:

  1. Explicitly specify SentryJSONProcessor initialization parameters (and, in turn, explicitly passing those to superclass initialization)
    • I'm assuming this is the main change that requires consideration?
  2. Add missing type annotations
  3. Add py.typed the package source code directory to mark that the structlog-sentry package supplies type information

Reviewer:

Tagging @aexvir for review (@kiwicom/platform is no longer valid 😞 ).

Thanks in advance!

Note:

Feel free to change commit messages, etc. as you see fit :)

Based on feedback from Mypy.
Resolves `TypeError`s thrown by Mypyc-compiled code as a result of
empty tuple assignment to an int-typed `level` parameter during
superclass initialization.
@TeoZosa TeoZosa force-pushed the fully-annotate-library-and-mark-as-pep-561-compatible branch from 8b735f1 to 070b940 Compare September 12, 2021 22:20
TeoZosa added a commit to TeoZosa/structlog-sentry-logger that referenced this pull request Sep 12, 2021
@paveldedik
Copy link
Collaborator

Hi @TeoZosa I'm sorry for the late response. If you are still interested in finishing this PR, there are quite a lot of changes that happened recently, see this PR:

For all changes see #79

Basically we released version 2.0.0b1 which I'm testing in production right now. It, for example, completely drops SentryJsonProcessor which should not be needed anymore (see the readme)

Let me know if you are still interested in finishing this PR.

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