Skip to content

PHPC-1145: Fix development build warnings for numeric type conversions #788

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

Merged
merged 1 commit into from
Mar 26, 2018

Conversation

kvwalker
Copy link
Contributor

@kvwalker kvwalker requested a review from jmikola March 26, 2018 17:13
@@ -41,7 +41,7 @@ static PHP_METHOD(CommandException, getResultDocument)
#if PHP_VERSION_ID >= 70000
resultdocument = zend_read_property(php_phongo_commandexception_ce, getThis(), ZEND_STRL("resultDocument"), 0, &rv TSRMLS_CC);
#else
resultdocument = zend_read_property(php_phongo_commandexception_ce, getThis(), ZEND_STRL("resultDocument"), 0 TSRMLS_CC);
resultdocument = zend_read_property(php_phongo_commandexception_ce, getThis(), ZEND_STRL("resultDocument"), 0 TSRMLS_CC);
Copy link
Member

@jmikola jmikola Mar 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume clang-format did this?

Even so, this doesn't appear to be related to PHPC-1145. Can we remove this?

@derickr: How can we best ensure that unrelated code formatting changes don't make it into PRs? Will that just require manual git checkout --/path/to/file to discard things?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sigh. Clang-format does other weird things, and they didn't reply to my issue(s) about it at all.

For now, doing the manual git checkout -- dance seems necessary.

@@ -96,7 +96,7 @@ PHP_METHOD(CommandFailedEvent, getOperationId)
return;
}

sprintf(int_as_string, "%" PHONGO_LONG_FORMAT, intern->operation_id);
sprintf(int_as_string, "%" PRId64, intern->operation_id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, operation_id and request_id are unsigned 64-bit integers. These should use PRIu64 instead of PRId64.

@@ -98,7 +98,7 @@ PHP_METHOD(CommandStartedEvent, getOperationId)
return;
}

sprintf(int_as_string, "%" PHONGO_LONG_FORMAT, intern->operation_id);
sprintf(int_as_string, "%" PRId64, intern->operation_id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PRIu64 in this file for operation_id and request_id.

@@ -75,7 +75,7 @@ PHP_METHOD(CommandSucceededEvent, getOperationId)
return;
}

sprintf(int_as_string, "%" PHONGO_LONG_FORMAT, intern->operation_id);
sprintf(int_as_string, "%" PRId64, intern->operation_id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PRIu64 in this file for operation_id and request_id.

@kvwalker kvwalker merged commit 225c0aa into mongodb:master Mar 26, 2018
kvwalker added a commit that referenced this pull request Mar 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants