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

trace_class add tracing for classmethod and staticmethdod #115

Conversation

EeroPaukkonen
Copy link
Contributor

@EeroPaukkonen EeroPaukkonen commented May 11, 2023

trace_class will trace classmethods, staticmethod by default.
This does not include property. I do not have strong opinion about property to either direction.

Slightly cleaned test folder structure.
Also not sure if enabling this by default would be issue for some 🤷

@gbbirkisson
Copy link
Contributor

I think classmethods and staticmethods should probably be traced by default. It is easy to opt out if you don't want that. I'm not sure about property though. Its probably overkill to trace that by default. You can always explicitly trace them if you want. Let me do some digging.

1 similar comment
@gbbirkisson
Copy link
Contributor

I think classmethods and staticmethods should probably be traced by default. It is easy to opt out if you don't want that. I'm not sure about property though. Its probably overkill to trace that by default. You can always explicitly trace them if you want. Let me do some digging.

@EeroPaukkonen
Copy link
Contributor Author

I think classmethods and staticmethods should probably be traced by default. It is easy to opt out if you don't want that. I'm not sure about property though. Its probably overkill to trace that by default. You can always explicitly trace them if you want. Let me do some digging.

Yea I also had reservations about property makes sense to keep it out.

@EeroPaukkonen EeroPaukkonen force-pushed the eerop/trace-class-include-classmethods-staticmethods-property-methods branch from 5a2b872 to 380a66e Compare August 3, 2023 05:30
@EeroPaukkonen EeroPaukkonen changed the title trace_class add tracing for classmethod, staticmethdod, property trace_class add tracing for classmethod, staticmethdod Aug 3, 2023
@EeroPaukkonen EeroPaukkonen marked this pull request as ready for review August 3, 2023 13:05
@EeroPaukkonen EeroPaukkonen changed the title trace_class add tracing for classmethod, staticmethdod trace_class add tracing for classmethod and staticmethdod Aug 3, 2023
Copy link
Contributor

@gbbirkisson gbbirkisson left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@EeroPaukkonen EeroPaukkonen merged commit 5a6fc08 into main Aug 4, 2023
5 checks passed
@EeroPaukkonen EeroPaukkonen deleted the eerop/trace-class-include-classmethods-staticmethods-property-methods branch August 4, 2023 07:29
EeroPaukkonen added a commit that referenced this pull request Aug 9, 2023
…)"

This reverts commit 5a6fc08. But leaves some of the tests.
EeroPaukkonen added a commit that referenced this pull request Aug 9, 2023
…)" (#183)

This reverts commit 5a6fc08. But leaves some of the tests.
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