Skip to content

Fix internal transaction method forwarding #1017

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 1 commit into from
Feb 6, 2024

Conversation

robsdedude
Copy link
Member

Forwarding methods shouldn't be decorated with functools.wraps as it not only copies the docs string and type annotations (desirable), but also __name__, __qualname__, and more (undesirable).

While wraps can be configured with what properties to copy, it turns out that there is no need to keep the doc strings in the transaction base class for the forwarded methods. Instead, they are moved to the public (forwarding) methods. This still keeps the code DRY.

Suggested in: #1015

Forwarding methods shouldn't be decorated with `functools.wraps` as it not only
copies the docs string and type annotations (desirable), but also `__name__`,
`__qualname__`, and more (undesirable).

While `wraps` can be configured with what properties to copy, it turns out that
there is no need to keep the doc strings in the transaction base class for the
forwarded methods. Instead, they are moved to the public (forwarding) methods.
This still keeps the code DRY.
Copy link

@RichardIrons-neo4j RichardIrons-neo4j left a comment

Choose a reason for hiding this comment

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

All makes sense.

@robsdedude robsdedude merged commit 6cd99ab into neo4j:5.0 Feb 6, 2024
@robsdedude robsdedude deleted the fix/transaction-method-forwarding branch February 6, 2024 15:29
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