Skip to content
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

Fixes for python omap method return values. #8603

Merged
merged 2 commits into from Apr 14, 2016
Merged

Conversation

rmechler
Copy link

Signed-off-by: Roland Mechler rmechler@cisco.com

Signed-off-by: Roland Mechler <rmechler@cisco.com>
@jdurgin
Copy link
Member

jdurgin commented Apr 14, 2016

lgtm, nice catch

@@ -2799,7 +2799,9 @@ returned %d, but should return zero on success." % (self.name, ret))
int _flags = flags

with nogil:
rados_write_op_operate(_write_op.write_op, self.io, _oid, &_mtime, _flags)
ret = rados_write_op_operate(_write_op.write_op, self.io, _oid, &_mtime, _flags)
if (ret != 0):
Copy link
Contributor

Choose a reason for hiding this comment

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

could also remove the parentheses here.

Copy link
Author

Choose a reason for hiding this comment

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

I used parentheses because I copied the style from similar code elsewhere in the file. There seems to be a mix of styles (parentheses vs not) in the file. So, should I (a) leave this as is, (b) remove the parentheses only on the code I've added, or (c) make the style consistent (no parentheses) everywhere in the file?

Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest (b) for this pr. A separate pr removing parens from ifs everywhere in pybind would be great. It's just a historical accident that kept getting copied.

Signed-off-by: Roland Mechler <rmechler@cisco.com>
@jdurgin
Copy link
Member

jdurgin commented Apr 14, 2016

passed python2 and 3 tests locally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants