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

PushEvent GetCommits() Method #2425

Closed
midnightconman opened this issue Jul 29, 2022 · 3 comments · Fixed by #2776
Closed

PushEvent GetCommits() Method #2425

midnightconman opened this issue Jul 29, 2022 · 3 comments · Fixed by #2776

Comments

@midnightconman
Copy link

midnightconman commented Jul 29, 2022

I would like to use an interface for this event in a local implementation, but unfortunately a getter for this type doesn't exist. Can we create a getter for PushEvent.GetCommits?

type PushEvent struct {
...
Commits      []*HeadCommit
...
}

func (p *PushEvent) GetCommits() []*HeadCommit {
    return p.Commits
}
@gmlewis
Copy link
Collaborator

gmlewis commented Jul 29, 2022

In general, getters are only provided for anything where a nil might be a problem (which is not the case for slices of anything), and all getters are auto-generated.

So if you want to add one for this, then I suppose we have a few options:

  1. add getters for all fields, but that seems wasteful
  2. modify the auto-generator to add a list of "special" getters that we want to always add (e.g. PushEvent.Commits)
  3. manually add a getter to the main source code and completely ignore the auto-generator

I'm kinda leaning toward option 2.

Thoughts?

@midnightconman
Copy link
Author

2 sounds good to me 😸 Let me take a look at the auto-generator code. Thank you!

@olibaa
Copy link
Contributor

olibaa commented Apr 30, 2023

I would like to propose some changes and request for a review.

I would appreciate it if you could check it out!

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 a pull request may close this issue.

3 participants