feat(subtract): added Subtract function to wire#382
feat(subtract): added Subtract function to wire#382giautm wants to merge 2 commits intogoogle:mainfrom
Conversation
giautm
force-pushed
the
g-subtract
branch
5 times, most recently
from
e4c29cc to
a5727b7
Compare
Signed-off-by: Giau. Tran Minh <hello@giautm.dev>
Signed-off-by: Giau. Tran Minh <hello@giautm.dev>
|
cc @jayzhuang, please give some comments or suggestions here. <3 |
|
any updates? |
|
I'm really in need of this PR. Is there any progress ? |
|
I also want this. But there are no review from the core team. So... |
|
@vangent PTAL |
|
Sorry, I don't maintain this anymore. Try @zombiezen or @eliben . |
nstandif
left a comment
nstandif
left a comment
There was a problem hiding this comment.
I am not golang core, but these are a few glaring issues I saw. Hope this helps.
| // whose element type is the struct type. | ||
| // - For a value, this is the type of the expression. | ||
| // - For an argument, this is the type of the argument. | ||
| // whose element type is the struct type. | ||
| // - For a value, this is the type of the expression. | ||
| // - For an argument, this is the type of the argument. |
There was a problem hiding this comment.
Existing comments shouldn't be modified, unless you're changing the function definition.
| // type S struct { | ||
| // MyFoo *Foo | ||
| // MyBar *Bar | ||
| // } | ||
| // var Set = wire.NewSet(wire.Struct(new(S), "MyFoo")) -> inject only S.MyFoo | ||
| // var Set = wire.NewSet(wire.Struct(new(S), "*")) -> inject all fields | ||
| // type S struct { | ||
| // MyFoo *Foo | ||
| // MyBar *Bar | ||
| // } | ||
| // var Set = wire.NewSet(wire.Struct(new(S), "MyFoo")) -> inject only S.MyFoo | ||
| // var Set = wire.NewSet(wire.Struct(new(S), "*")) -> inject all fields |
There was a problem hiding this comment.
Revert comment spacing changes, since you didn't touch this code.
| // type S struct { | ||
| // MyFoo Foo | ||
| // MyBar Bar | ||
| // } | ||
| // type S struct { | ||
| // MyFoo Foo | ||
| // MyBar Bar | ||
| // } | ||
| // | ||
| // func NewStruct() S { /* ... */ } | ||
| // var Set = wire.NewSet(wire.FieldsOf(new(S), "MyFoo", "MyBar")) | ||
| // func NewStruct() S { /* ... */ } | ||
| // var Set = wire.NewSet(wire.FieldsOf(new(S), "MyFoo", "MyBar")) | ||
| // | ||
| // or | ||
| // or | ||
| // | ||
| // func NewStruct() *S { /* ... */ } | ||
| // var Set = wire.NewSet(wire.FieldsOf(new(*S), "MyFoo", "MyBar")) | ||
| // func NewStruct() *S { /* ... */ } | ||
| // var Set = wire.NewSet(wire.FieldsOf(new(*S), "MyFoo", "MyBar")) | ||
| // | ||
| // If the structType argument is a pointer to a pointer to a struct, then FieldsOf | ||
| // additionally provides a pointer to each field type (e.g., *Foo and *Bar in the | ||
| // example above). | ||
| // If the structType argument is a pointer to a pointer to a struct, then FieldsOf | ||
| // additionally provides a pointer to each field type (e.g., *Foo and *Bar in the | ||
| // example above). |
There was a problem hiding this comment.
Comment spacing should be reverted, since you didn't modify anything here.
|
|
||
| func Subtract(...interface{}) ProviderSet { | ||
| return ProviderSet{} | ||
| } |
There was a problem hiding this comment.
Follow the commenting conventions of previous examples. changing spaces to tabs, in this case, is probably OK.
| return nil, []error{notePosition(exprPos, errors.New("unknown pattern"))} | ||
| } | ||
|
|
||
| func (oc *objectCache) filterType(set *ProviderSet, t types.Type) []error { |
There was a problem hiding this comment.
briefly describe what this function does.
| return nil | ||
| } | ||
|
|
||
| func (oc *objectCache) processSubtract(info *types.Info, pkgPath string, call *ast.CallExpr, args *InjectorArgs, varName string) (interface{}, []error) { |
There was a problem hiding this comment.
Briefly describe the function and the args passed.
|
@zombiezen PTAL |
|
I do not have the time to shepherd in a new feature in Wire, sorry. (FWIW, the comment spacing changes are fine: gofmt changed within the last couple years and does a better job formatting lists and such.) |
|
@jayzhuang I saw your comment in the related issue, are you open to reviewing this change? |
7 participants
Fixes #8