[GODRIVER-572] TransformDocument should document how it handles nil Created: 18/Sep/18 Updated: 28/Oct/23 Resolved: 21/Feb/19 |
|
| Status: | Closed |
| Project: | Go Driver |
| Component/s: | Documentation |
| Affects Version/s: | None |
| Fix Version/s: | 1.0.0-rc1 |
| Type: | Improvement | Priority: | Minor - P4 |
| Reporter: | David Golden | Assignee: | Isabella Siu (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Epic Link: | GoDocs for 1.0 |
| Description |
|
nil is equivalent to an empty document, but this is not documented. |
| Comments |
| Comment by Githook User [ 21/Feb/19 ] |
|
Author: {'name': 'Isabella Siu', 'email': 'isabella.siu@10gen.com', 'username': 'iwysiu'}Message: Change-Id: I1dd743891dd2ca49ebee0b76fa15f5ea738813a4 |
| Comment by Isabella Siu (Inactive) [ 20/Feb/19 ] |
|
code review: https://review.gerrithub.io/c/mongodb/mongo-go-driver/+/445618 |
| Comment by David Golden [ 07/Jan/19 ] |
|
I don't think that's enough. Empty interface is like void*. Just saying "non-nil" isn't enough. We should be positively listing what types/interfaces are allowed. (Doing it in doc.go seems fine, though.) |
| Comment by Divjot Arora (Inactive) [ 07/Jan/19 ] |
|
I think adding it to doc.go is good enough. Just a small note like "all methods in collection.go that take params of type interface{} will return ErrNilDocument if nil is passed in" |
| Comment by David Golden [ 07/Jan/19 ] |
|
As far as I can see, there is zero documentation in the mongo package about what types are acceptable to the empty interface parameters that are intended to take "documents". I think now that TransformDocument is private, there still needs to be such documentation somewhere. (See for example, this terminology section in Perl driver documentation). So perhaps this just needs to be renamed to "Document what types are allowed as documents for empty interface parameters". |
| Comment by Kristofer Brandow (Inactive) [ 07/Jan/19 ] |
|
We have ErrNilDocument, but if that's not enough we can call it out in the main documentation in the doc.go file. I don't think we should add it to every single CRUD method. |
| Comment by Divjot Arora (Inactive) [ 07/Jan/19 ] |
|
yep - not sure if we should make a new ticket for the docs or just modify this ticket to add documentation for all CRUD functions to say when nil is and isn't allowed. |
| Comment by Kristofer Brandow (Inactive) [ 07/Jan/19 ] |
|
It's transformDocument itself that returns an error if you provide it with nil, so it'll return an error for inserts and finds alike. |
| Comment by Divjot Arora (Inactive) [ 07/Jan/19 ] |
|
Previous comment was incorrect. transformDocument() now checks to see if the value passed in is nil and returns an error if it is. This behavior isn't document anywhere, though. At the very least, the documentation for Find() should say that a nil filter isn't allowed. |
| Comment by Kristofer Brandow (Inactive) [ 07/Jan/19 ] |
|
Since |
| Comment by David Golden [ 11/Oct/18 ] |
|
It's possible there needs to be a top-level documentation section on how empty interface documents are handled across the API (assuming that behavior is universal). |
| Comment by Divjot Arora (Inactive) [ 11/Oct/18 ] |
|
From what I understand, the TransformDocument function previously used by collection.go has been replaced by a transformDocument function defined in mongo/mongo.go. The documentation for the collection.go functions should be updated to reflect this because many of them say "See TransformDocument for the list of valid types for documents." I'm not sure how docs for private functions work. Should transformDocument be documented for users or should each collection.go function that uses it specify the types that can be passed to it? |