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

Implement a dig.As ProvideOption #252

Closed
wants to merge 1 commit into from
Closed

Implement a dig.As ProvideOption #252

wants to merge 1 commit into from

Conversation

abhinav
Copy link
Collaborator

@abhinav abhinav commented Nov 14, 2019

This brings back #233.

Per #235 (review),
the issues we need to resolve are,

  1. dig.As seems to indicate that it's a total override of the provided
    type. The way it currently works is it appends other interfaces on
    top of whatever the constructor already returns
  2. semantics of dig.Provide(func New() (Foo, io.Reader, error), dig.As(new(io.Writer).
    It currently fails due to inability to case reader to writer, but
    we'd want some extra validation here. Perhaps dig.As is only
    supported for a single type, error tuple?

Closes #197

@codecov
Copy link

codecov bot commented Nov 14, 2019

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.47%. Comparing base (4fb70ce) to head (287b330).
Report is 123 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #252      +/-   ##
==========================================
+ Coverage   98.40%   98.47%   +0.06%     
==========================================
  Files          10       10              
  Lines        1130     1179      +49     
==========================================
+ Hits         1112     1161      +49     
  Misses         13       13              
  Partials        5        5              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@abhinav abhinav requested a review from twilly November 14, 2019 19:56
@glibsm
Copy link
Collaborator

glibsm commented Nov 14, 2019

semantics of dig.Provide(func New() (Foo, io.Reader, error), dig.As(new(io.Writer).
It currently fails due to inability to case reader to writer, but
we'd want some extra validation here. Perhaps dig.As is only
supported for a single type, error tuple?

I see the following:

  1. As you said, only restrict to one type and error (has edge cases)

  2. dig.As can take a vararg which gets matched to the positional arguments (minus error). Also has edge cases and is not the way the current docs are written. Docs allow for multiple casts of the same return type (is that a required usecase?)

  3. ??? Maybe there is some sort of a matrix solution where you're able to cast as many params in as many ways as you want (probably requires dig.As(...) to be nestable somehow)

@abhinav abhinav changed the base branch from abg/revert-as to master November 14, 2019 21:21
This brings back #233.

Per #235 (review),
the issues we need to resolve are,

1. `dig.As` seems to indicate that it's a total override of the provided
   type. The way it currently works is it appends other interfaces on
   top of whatever the constructor already returns
2. semantics of `dig.Provide(func New() (Foo, io.Reader, error), dig.As(new(io.Writer)`.
   It currently fails due to inability to case reader to writer, but
   we'd want some extra validation here. Perhaps `dig.As` is only
   supported for a single type, error tuple?

Closes #197
abhinav added a commit that referenced this pull request Nov 14, 2019
This sets up release v1.8.0. The only change in this release is the
migration to Go modules.

We backed out #233; we'll try it again in #252.
@abhinav abhinav mentioned this pull request Nov 14, 2019
abhinav added a commit that referenced this pull request Nov 14, 2019
This sets up release v1.8.0. The only change in this release is the
migration to Go modules.

We backed out #233; we'll try it again in #252.
@CLAassistant
Copy link

CLAassistant commented Jun 13, 2020

CLA assistant check
All committers have signed the CLA.

@abhinav
Copy link
Collaborator Author

abhinav commented Sep 18, 2021

Superseded by #293

@abhinav abhinav closed this Sep 18, 2021
@abhinav abhinav deleted the abg/as branch September 18, 2021 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Allow to specify an interface when providing a constructor
4 participants