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

gen-accessors: skip Client struct and unexported fields #794

Merged
merged 4 commits into from Dec 2, 2017

Conversation

sahildua2305
Copy link
Member

Make gen-accessors.go skip certain struct and fields while creating
accessor methods for structs and fields. Basically, now we have two more
blacklists (blacklistStruct and blacklistField) respectively for
blacklisted structs and fields.

So, for every struct, it should first check and skip if the struct
is in blacklist.
Then, for every field, it should first check and skip if the struct
is in blacklist.

Fixes #778

@googlebot googlebot added the cla: yes Indication that the PR author has signed a Google Contributor License Agreement. label Nov 23, 2017
Make gen-accessors.go skip certain struct and fields while creating
accessor methods for structs and fields. Basically, now we have two more
blacklists (blacklistStruct and blacklistField) respectively for
blacklisted structs and fields.

So, for every struct, it should first check and skip if the struct
is in blacklist.
Then, for every field, it should first check and skip if the struct
is in blacklist.
- Remove "service" from list of blacklisted fields because that was a
  mistake in the previous commit.
- Add "service" to list of blacklisted structs because it  would make
  more sense to not generate accessors for its fields.
- Remove all debug printf statements.
// accessors.
blacklistStruct = map[string]bool{
"Client": true,
"service": true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We never want to generate accessors for unexported types and fields, right?

Then I think we can/should simplify this. Instead of manually listing service struct here, and client field below, just add a general rule to skip unexported identifiers.

Then, blacklistStruct will contain just Client, and blacklistField can go away completely.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah! Thanks for the hint. Made changes accordingly. Please review again.

@dmitshur
Copy link
Member

dmitshur commented Dec 2, 2017

Looks good so far, thanks @sahildua2305! I'll apply some comment formatting improvements and code simplifications, and then merge.

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@googlebot googlebot added cla: no and removed cla: yes Indication that the PR author has signed a Google Contributor License Agreement. labels Dec 2, 2017
Copy link
Member

@dmitshur dmitshur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks again!

@dmitshur dmitshur merged commit fbfee05 into google:master Dec 2, 2017
nbareil pushed a commit to nbareil/go-github that referenced this pull request May 1, 2018
…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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants