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

[v1.23] Testing the new iter package with gh-iter #3209

Closed
enrichman opened this issue Jul 16, 2024 · 7 comments · Fixed by #3212
Closed

[v1.23] Testing the new iter package with gh-iter #3209

enrichman opened this issue Jul 16, 2024 · 7 comments · Fixed by #3212
Assignees

Comments

@enrichman
Copy link
Contributor

The incoming v1.23 Go version will introduce the iter package, and tried to implement a generic wrapper around the client.
It support pagination in a transparent way, with generics:

// init your Github client
client := github.NewClient(nil)

// create an iterator, and start looping! 🎉
users := ghiter.NewFromFn(client.Users.ListAll)
for u := range users.All() {
	fmt.Println(*u.Login)
}

https://github.com/enrichman/gh-iter

Maybe it can provides some hints about embedding this new feature into the official client, or if you are more about keeping the two separates then you are probably aware about some edge cases I'm not aware of.

For example I've read about some annoyance about the setting of the query parameters of the link headers in some issues. I tried to use the reflect package to dynamically set those in the provided options, checking the url tag of the option fields.

Looking for feedbacks!

@gmlewis
Copy link
Collaborator

gmlewis commented Jul 18, 2024

That looks very cool, @enrichman ! Thank you for putting that together!

Let's do this... how about you create a PR that updates the README.md in this repo to point to your repo as a new way to iterate over endpoint pagination with maybe a small example, and let's see how users of this repo pick it up.

I'm honestly thinking that if it becomes hugely popular, that there still may not be any real need to pull it into this repo... at least I can't think of any benefit off-hand.

How does that sound to you?

We can also leave this issue open for others to comment on this idea while you think about updating this repo's README.

@enrichman
Copy link
Contributor Author

That would be great, thanks @gmlewis for the opportunity!

@marcofranssen
Copy link

It would be great if the gh-iter package could be added natively in here.

Why?

Because of the following: enrichman/gh-iter#1

Because there is a hard dependency on the specific go-github version any new major release would have to be coordinated with the gh-iter package. Including the gh-iter package natively in go-github would get rid of that problem.

Downside is that go-github now will have a hard minimum version requirement of go > 1.23. However that might be fine to just make that as a requirement for the next major go-github release.

@gmlewis
Copy link
Collaborator

gmlewis commented Sep 23, 2024

This repo has historically officially supported the most recent two minor versions of the Go compiler, so in this case, we need to continue to support Go 1.22.x and Go 1.23.x.

So we have some options:

  1. Make v63.0.0, v64.0.0 v65.0.0 releases of gh-iter that match the same version of go-github (this is the easiest solution by far)
  2. Wait until Go 1.24.0 is released, then create a PR to incorporate gh-iter directly into go-github (without any extra dependencies other than the Go standard library of course)
  3. Remove the dependency upon Go 1.23 from gh-iter and incorporate it into go-github now with a new PR.

Thoughts? (I vote for option 1 in case it wasn't obvious.)

@gmlewis gmlewis reopened this Sep 23, 2024
@enrichman
Copy link
Contributor Author

Thanks for your opinion. I think that at the moment option 1 is the easiest.

It will be nice to incorporate the package into the library when 1.24 will be out, if that's possible.

@gmlewis
Copy link
Collaborator

gmlewis commented Sep 23, 2024

Sounds good. Closing issue for now and we can revisit when Go 1.24 is released.

@gmlewis gmlewis closed this as completed Sep 23, 2024
@marcofranssen
Copy link

I vote for option 2 once Go 1.24 is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants