Skip to content

Add WithCallback Feature #3681

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

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

Conversation

Not-Dhananjay-Mishra
Copy link

This PR implements the feature requested in [#3658] to allow configurable telemetry for the Client.

Modifications -

  • WithCallback(callback func()) was added. The callback will be set by the user.
  • The callback execute just before the client.bareDo inside client.Do.
  • Allows for chaining because it returns the same Client instance.
  • Added new Callback field in the Client struct of type func(), which default set to nil during initialize().

Example Code -

client := github.NewClient(nil).WithCallback(func() {
fmt.Println("Callback executed")
})

This enables users to attach custom telemetry or logging kind of things to API calls in a flexible, user friendly manner.
Btw this is my first PR
Happy to get any feedback or suggestions to improve it

@gmlewis gmlewis changed the title Added WithCallback Feature [#3658] Add WithCallback Feature [#3658] Aug 16, 2025
@Not-Dhananjay-Mishra
Copy link
Author

Corrected the typo that caused the lint issue. I’m still learning, so thank you

@gmlewis gmlewis changed the title Add WithCallback Feature [#3658] Add WithCallback Feature Aug 16, 2025
Copy link
Contributor

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @Not-Dhananjay-Mishra - but I don't think this fully satisfies what is being requested in #3658.

Additionally, discussion has not completed in that issue, as there is an outstanding request to the original author to further clarify their request, so it is premature to be writing any PR for that issue yet.

Let's hold off on this PR until we get some more clarity, please.

@gmlewis gmlewis added the DO NOT MERGE Do not merge this PR. label Aug 16, 2025
@Not-Dhananjay-Mishra
Copy link
Author

Thanks @gmlewis for the clarification! I will hold off on the PR. I appreciate the guidance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO NOT MERGE Do not merge this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants