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

gen-accessors should skip Client, service structs and unexported fields. #778

Closed
dmitshur opened this Issue Nov 12, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@dmitshur
Member

dmitshur commented Nov 12, 2017

There are two issues with gen-accessors.go generating accessors it should not be (/cc @gmlewis FYI). It's relatively harmless, but should be fixed.

First, gen-accessors.go does not skip unexported fields, which causes it to generate accessors that it shouldn't. For example:

// Getclient returns the client field.
func (s *service) Getclient() *Client {
	if s == nil {
		return nil
	}
	return s.client
}

Second, it does not skip certain structs that it should skip, including Client, service, and perhaps others. It should only generate accessors for structs that are mapped to GitHub objects. For example:

image

image

@sahildua2305

This comment has been minimized.

Show comment
Hide comment
@sahildua2305

sahildua2305 Nov 14, 2017

Collaborator

@shurcooL I'd like to tackle this. What do you think about having an extended list of blacklisted structs to avoid while generating accessor methods?

Collaborator

sahildua2305 commented Nov 14, 2017

@shurcooL I'd like to tackle this. What do you think about having an extended list of blacklisted structs to avoid while generating accessor methods?

@dmitshur

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Nov 15, 2017

Member

Thanks!

I think adding a list of blacklisted structs is the way to go. There's only a couple of them.

Member

dmitshur commented Nov 15, 2017

Thanks!

I think adding a list of blacklisted structs is the way to go. There's only a couple of them.

@dmitshur dmitshur closed this in #794 Dec 2, 2017

dmitshur added a commit that referenced this issue Dec 2, 2017

Remove generated accessors for Client struct and unexported fields. (#…
…794)

This change makes gen-accessors.go skip the following structs and fields
from consideration when generating accessor methods:

1.	Unexported structs and fields are skipped. They're unexported,
	and shouldn't have accessors.
2.	Client struct is explicitly blacklistd and skipped. It was not meant
	to have accessors, and it doesn't need them.

The generated accessors that are removed were added relatively recently,
and it's not likely they were used since that time.

Resolves #778.

nbareil pushed a commit to nbareil/go-github that referenced this issue May 1, 2018

Remove generated accessors for Client struct and unexported fields. (g…
…oogle#794)

This change makes gen-accessors.go skip the following structs and fields
from consideration when generating accessor methods:

1.	Unexported structs and fields are skipped. They're unexported,
	and shouldn't have accessors.
2.	Client struct is explicitly blacklistd and skipped. It was not meant
	to have accessors, and it doesn't need them.

The generated accessors that are removed were added relatively recently,
and it's not likely they were used since that time.

Resolves google#778.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment