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

Bug: Issues.ListByRepo Repository field is always nil #3159

Open
erezrokah opened this issue May 6, 2024 · 5 comments
Open

Bug: Issues.ListByRepo Repository field is always nil #3159

erezrokah opened this issue May 6, 2024 · 5 comments

Comments

@erezrokah
Copy link
Contributor

func (s *IssuesService) ListByRepo(ctx context.Context, owner string, repo string, opts *IssueListByRepoOptions) ([]*Issue, *Response, error) {
returns an Issue struct with a Repository field but it's always nil.

Looks like that's expected based on GitHub docs https://docs.github.com/rest/issues/issues#list-repository-issues

Doing gh api repos/google/go-github/issues doesn't return repository as well

@gmlewis
Copy link
Collaborator

gmlewis commented May 11, 2024

It sounds to me like you are saying that the GitHub v3 API should return a repo with the endpoint.

However, this repo is a Go client library to the GitHub v3 API, and not the official v3 API itself.

Therefore, this bug report is in the wrong place and needs to be submitted to the official GitHub tech support team.

I'm going to go ahead and close this bug report, but feel free to refer to it when contacting the GitHub tech support team.

@gmlewis gmlewis closed this as completed May 11, 2024
@erezrokah
Copy link
Contributor Author

Hi 👋 thanks for the reply. Sorry for not being clear, I'm saying it should not return the repository based on the rest API schema from the GitHub documentation. So I believe the Go struct should not include that field.

@gmlewis
Copy link
Collaborator

gmlewis commented May 11, 2024

Hi 👋 thanks for the reply. Sorry for not being clear, I'm saying it should not return the repository based on the rest API schema from the GitHub documentation. So I believe the Go struct should not include that field.

Oh! Gotcha. Now I understand. Sorry for being a bit slow.

It turns out in this repo that we reuse a great number of structs when they are mostly similar so that we don't have to define custom structs for every possible use case. As a result, we only remove fields when we are 100% confident that a field is not used elsewhere.

Feel free to open up a PR if you believe that is the case in this instance. I'll re-open the issue so that you or someone else can confirm or deny that the field is not used anywhere else and can indeed be removed as suggested.

@gmlewis gmlewis reopened this May 11, 2024
@erezrokah
Copy link
Contributor Author

It turns out in this repo that we reuse a great number of structs when they are mostly similar so that we don't have to define custom structs for every possible use case

👍 I believe we noticed this also for Repositories.ListByOrg and CustomProperties (only available on Get repo and not List).

Are you open to using the Open API schema to generate the structs per API endpoint?
I think it would be quite hard to manually flush out all the differences.
I'm not saying we should switch everything in one go, but maybe try to see what it gives in regards with the diff from the current code.

@gmlewis
Copy link
Collaborator

gmlewis commented May 14, 2024

That sounds like an interesting proposal, and I'm not yet ready to commit to it, but would love to see some experimental results first if you are willing to put something together in order to see what it really looks like.

If it is too disruptive for current users of this repo, then we probably will pass.
But if it appears to be really useful and beneficial, then we can take the next step(s).

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

No branches or pull requests

2 participants