-
Notifications
You must be signed in to change notification settings - Fork 208
PHPC-359 and PHPC-801: RC, RP, and WC BSON serialization #424
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some small changes, LGTM otherwise.
@@ -864,10 +907,10 @@ void php_phongo_read_preference_to_zval(zval *retval, const mongoc_read_prefs_t | |||
} | |||
|
|||
if (!bson_empty0(tags)) { | |||
/* Use PHONGO_TYPEMAP_NATIVE_ARRAY for the root type since tags is an | |||
* array; however, inner documents and arrays can use the default. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we be explicit about the document_type? i.e., by setting it to something like:
state.map.document_type = PHONGO_TYPEMAP_DEFAULT;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PHONGO_BSON_STATE_INITIALIZER
is already the default, which we use for the state
intialization. This should suffice.
for (zend_hash_internal_pointer_reset_ex(ht_data, &pos); | ||
zend_hash_get_current_data_ex(ht_data, (void **) &tagSet, &pos) == SUCCESS; | ||
zend_hash_move_forward_ex(ht_data, &pos)) { | ||
if (Z_TYPE_PP(tagSet) == IS_ARRAY) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like whitespace issues here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is correct. The for
components are all indented five spaces and the if
(and lines below) are indented with a tab, since they're within the for
block.
@@ -100,6 +100,62 @@ static bool php_phongo_manager_merge_context_options(zval *zdriverOptions TSRMLS | |||
return true; | |||
} | |||
|
|||
static void php_phongo_manager_prep_tagsets(zval *options TSRMLS_DC) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like the comment that you used for RPs here too.
https://jira.mongodb.org/browse/PHPC-359
https://jira.mongodb.org/browse/PHPC-801