Skip to content

Add missing memo kwargs to callback protocols #666

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

Merged
merged 5 commits into from
Feb 5, 2021
Merged

Conversation

nolar
Copy link
Owner

@nolar nolar commented Feb 5, 2021

Add missing memo kwargs to callback protocols and make them part of the public interface (docs, tests).

In addition, resolve circular imports:

  • callbacks require containers (now: memos) for the kwarg memo.
  • containers require handlers for HandlerId & daemon handler.
  • handlers require callbacks for callback protocols.

Previously, it was not a problem because callbacks did not declare memo kwargs by mistake, and therefore did not import containers.

The chain is broken at the callbackscontainers import: it is now callbacksmemos.

nolar added 5 commits February 5, 2021 22:39
Specifically:

* `callbacks` require `containers` (now: memos) for the kwarg `memo`.
* `containers` require `handlers` for `HandlerId` & daemon handler
* `handlers` require `callbacks` for callback protocols

Previously, it was not a problem because `callbacks` did not declare `memo` kwargs by mistake, and therefore did not import `containers`.

The chain is broken at the `callbacks`→`containers` import: it is now `callbacks`→`memos`.

Signed-off-by: Sergey Vasilyev <[email protected]>
Signed-off-by: Sergey Vasilyev <[email protected]>
@nolar nolar added the refactoring Code cleanup without new features added label Feb 5, 2021
@nolar nolar merged commit a45d81a into master Feb 5, 2021
@nolar nolar deleted the missing-memos branch February 5, 2021 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Code cleanup without new features added
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant