make Modify method return result and err #229
Conversation
jamesgoodhouse
force-pushed
the
handle_modify_response
branch
5 times, most recently
from
6a98fad to
2de97af
Compare
|
Any thoughts on this @johnweldon? |
|
@johnweldon I kind of figured that'd be the case. I'll make a new method called |
jamesgoodhouse
force-pushed
the
handle_modify_response
branch
4 times, most recently
from
38527b1 to
5ecadec
Compare
|
@johnweldon I implemented the |
jamesgoodhouse
force-pushed
the
handle_modify_response
branch
from
a8cf7ba to
b1f4816
Compare
|
@johnweldon all done |
| func (l *Conn) ModifyWithResult(modifyRequest *ModifyRequest) (*ModifyResult, error) { | ||
| packet := ber.Encode(ber.ClassUniversal, ber.TypeConstructed, ber.TagSequence, nil, "LDAP Request") | ||
| packet.AppendChild(ber.NewInteger(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, l.nextMessageID(), "MessageID")) | ||
| packet.AppendChild(modifyRequest.encode()) |
There was a problem hiding this comment.
Does this line compile for you? There is no encode() method on the ModifyRequest type.
There was a problem hiding this comment.
@johnweldon: ah, there's not. i copy/pasta'd the code from when it was just modifying the original Modify method. I'll look into this and get it fixed.
There was a problem hiding this comment.
@johnweldon so looks like the encode() method was removed in https://github.com/go-ldap/ldap/pull/232/files#diff-25200df334664e32a437f35969897352L105. It's going to take me a bit to figure out what appears to be the replacement, appendTo.
It looks like it's a matter of replacing the encode stuff with l.doRequest(...). Does that sound right?
There was a problem hiding this comment.
Ah, yes this changed in PR #232 - there are some overlapping changes so you may want to review that PR as you update yours.
Thanks again
| func (l *Conn) ModifyWithResult(modifyRequest *ModifyRequest) (*ModifyResult, error) { | ||
| packet := ber.Encode(ber.ClassUniversal, ber.TypeConstructed, ber.TagSequence, nil, "LDAP Request") | ||
| packet.AppendChild(ber.NewInteger(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, l.nextMessageID(), "MessageID")) | ||
| packet.AppendChild(modifyRequest.encode()) |
|
Feel free to submit a new PR if you decide to pursue these changes. Thanks for your contribution. |
This implements a proposal I filled in #228. This unfortunately would be a breaking change for the existing code, so I'm not sure how you would like to handle that. I mentioned in my proposal that an alternative would be adding an auxiliary Modify function, something like
ModifyWithResponse(...), but i'll wait for some feedback before pivoting to that.