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

Make hooks settings generic #1428

Merged
merged 7 commits into from
May 13, 2020
Merged

Make hooks settings generic #1428

merged 7 commits into from
May 13, 2020

Conversation

jd
Copy link
Contributor

@jd jd commented May 12, 2020

refactor: move hooks module out of settings

This move the hooks class away of settings so it can be used by non-settings
code without import statement looking weird.

It also hides the module as being private.

refactor(hooks): make emit a public method

refactor(hooks): leverage attr to simplify code

refactor(hooks): remove span argument

The only user of this API seems to always send a span and nothing breaks while
removing this check.

chore: add _hooks to black

feat(hooks): make deregister take a hook argument

That makes sure we are able to deregister a hook for a particular event.

jd added 6 commits May 12, 2020 15:17
This move the hooks class away of settings so it can be used by non-settings
code without import statement looking weird.

It also hides the module as being private.
The only user of this API seems to always send a span and nothing breaks while
removing this check.
That makes sure we are able to deregister a hook for a particular event.
Kyle-Verhoog
Kyle-Verhoog previously approved these changes May 12, 2020
Copy link
Member

@Kyle-Verhoog Kyle-Verhoog left a comment

Choose a reason for hiding this comment

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

👍 nice, all the changes make sense.

It didn't make sense to have the implementation as public as the user never has to interact with the Hooks object directly.

@Kyle-Verhoog Kyle-Verhoog merged commit a19ccc8 into DataDog:master May 13, 2020
@jd jd deleted the rework-hooks branch May 13, 2020 07:35
@Kyle-Verhoog Kyle-Verhoog added this to the 0.38.0 milestone May 15, 2020
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.

None yet

2 participants