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

wire: FieldsOf now provides a pointer to the field type as well as the actual field type #209

Merged
merged 2 commits into from Sep 3, 2019

Conversation

vangent
Copy link
Contributor

@vangent vangent commented Aug 27, 2019

Fixes #208.

@vangent vangent requested a review from shantuo August 27, 2019 20:21
@googlebot googlebot added the cla: yes Google CLA has been signed! label Aug 27, 2019
@go-cloud-bot
Copy link

go-cloud-bot bot commented Aug 27, 2019

Please edit the title of this pull request with the name of the affected component, or "all", followed by a colon, followed by a short summary of the change.

@vangent vangent changed the title wire.FieldsOf now provides a pointer to the field type as well as the actual field type wire: FieldsOf now provides a pointer to the field type as well as the actual field type Aug 27, 2019
docs/guide.md Outdated
@@ -231,12 +231,13 @@ have a provider in the same set that provides the concrete type.
[type identity]: https://golang.org/ref/spec#Type_identity
[return concrete types]: https://github.com/golang/go/wiki/CodeReviewComments#interfaces

### Struct Providers
### Injected Structs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reworded this section a bit; I found it a bit confusing since FieldsOf is really using structs as providers, whereas Struct is injecting a struct. Feedback welcome.

Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding of the term "provider" in wire means the "initialization function" that provides a certain type. For wire.Struct, the provider is the generated code that populates the struct, for wire.FieldsOf, the provider is the generated code that returns the asked fields. In this case the title "Struct Providers" looks accurate to me. While one can argue the title below for FieldsOf is confusing, I would change it to something like "Struct Fields Providers" or "Injecting Fields of Structs".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PTAL.

Copy link
Contributor

Choose a reason for hiding this comment

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

Forgot to push?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, sorry, done.

docs/guide.md Outdated
@@ -231,12 +231,13 @@ have a provider in the same set that provides the concrete type.
[type identity]: https://golang.org/ref/spec#Type_identity
[return concrete types]: https://github.com/golang/go/wiki/CodeReviewComments#interfaces

### Struct Providers
### Injected Structs
Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding of the term "provider" in wire means the "initialization function" that provides a certain type. For wire.Struct, the provider is the generated code that populates the struct, for wire.FieldsOf, the provider is the generated code that returns the asked fields. In this case the title "Struct Providers" looks accurate to me. While one can argue the title below for FieldsOf is confusing, I would change it to something like "Struct Fields Providers" or "Injecting Fields of Structs".

internal/wire/parse.go Show resolved Hide resolved
internal/wire/parse.go Show resolved Hide resolved
@vangent vangent requested a review from shantuo August 28, 2019 02:08
@vangent vangent merged commit 2b7d120 into google:master Sep 3, 2019
@vangent vangent deleted the fieldptr branch September 3, 2019 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA has been signed!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow FieldsOf to provide a pointer type
3 participants