← 输入框|github.com|刷新
Skip to content

Add WithCallback Feature - #3681

Closed
Not-Dhananjay-Mishra wants to merge 3 commits into
google:masterfrom
Not-Dhananjay-Mishra:withcallback
Closed

Not-Dhananjay-Mishra wants to merge 3 commits into
google:masterfrom
Not-Dhananjay-Mishra:withcallback

Conversation

@Not-Dhananjay-Mishra

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor 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

@gmlewis gmlewis left a comment

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
Contributor 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