You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by "pivotal-jbarrett (GitHub)" <gi...@apache.org> on 2018/09/11 23:11:30 UTC
[GitHub] [geode-native] pivotal-jbarrett opened pull request #349:
GEODE-5698: Fixes shared_ptr access.
[ Full content available at: https://github.com/apache/geode-native/pull/349 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#349: GEODE-5698: Fixes shared_ptr access.
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
This looks like the formatter just cleaned up this comment. I'd rather stay away from it in this commit. I used `clang-tidy` to automate the fixes and `clang-format` to cleanup the formatting after.
[ Full content available at: https://github.com/apache/geode-native/pull/349 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on issue #349:
GEODE-5698: Fixes shared_ptr access.
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
@pdxcodemonkey Any objects to keeping the automated minimal changes. I like your other refactoring suggestions but I'd rather do that as a result of other non-automated work.
[ Full content available at: https://github.com/apache/geode-native/pull/349 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#349: GEODE-5698: Fixes shared_ptr access.
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Again, this was mostly automated. I'd like to keep it that way.
[ Full content available at: https://github.com/apache/geode-native/pull/349 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on pull request #349:
GEODE-5698: Fixes shared_ptr access.
Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
Same here, `== true` is redundant
[ Full content available at: https://github.com/apache/geode-native/pull/349 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on pull request #349:
GEODE-5698: Fixes shared_ptr access.
Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
The test code seems very fond of verbose comparisons like this - how about just `(*pIPtr != *newPiPtr)`?
[ Full content available at: https://github.com/apache/geode-native/pull/349 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey closed pull request #349:
GEODE-5698: Fixes shared_ptr access.
Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
[ pull request closed by pdxcodemonkey ]
[ Full content available at: https://github.com/apache/geode-native/pull/349 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on pull request #349:
GEODE-5698: Fixes shared_ptr access.
Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
Any reason we can't just delete this commented-out code?
[ Full content available at: https://github.com/apache/geode-native/pull/349 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on issue #349:
GEODE-5698: Fixes shared_ptr access.
Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
Makes sense, I'm fine with it as is.
[ Full content available at: https://github.com/apache/geode-native/pull/349 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#349: GEODE-5698: Fixes shared_ptr access.
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Again, this was mostly automated. I'd like to keep it that way.
[ Full content available at: https://github.com/apache/geode-native/pull/349 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org