PHPC-1490: Add support for var_export() and __set_state() in CursorId.c #1231
Conversation
| ADD_ASSOC_INT64_AS_STRING(&retval, "id", intern->id); | ||
| #else | ||
| ADD_ASSOC_LONG_EX(&retval, "id", intern->id); | ||
| #endif |
There was a problem hiding this comment.
We should keep this around so var_dump() behavior isn't changed by this PR. That should entail copying this to php_phongo_cursorid_get_properties_hash and placing it within a conditional dependent on is_debug.
There was a problem hiding this comment.
LGTM.
When squashing and merging, you can disregard all of the individual commit messages and just use the top-level commit with "PHPC-1490: Add support for var_export() and __set_state() in CursorId.c" and the PR number.
I noticed most of the commits repeat the same message and issue number. In the future, it'd be clearer to use messages that reflect the changes within since we expect to squash everything at merge time. For example, the message for 7fb5706 could have just mentioned that the expected exception class was being fixed. If the entire PR pertains to a single issue, prefixing each commit with an issue also shouldn't be necessary since we're going to squash everything.
This is in contrast to mongodb/mongo-php-library#834, which will end up having multiple commits specific tickets and not be squashed into one at merge time.
2 participants
https://jira.mongodb.org/browse/PHPC-1490