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/20 19:55:17 UTC

[GitHub] [geode-native] pivotal-jbarrett opened pull request #359: GEODE-5744: Use fixed width int types.


[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] igodwin commented on pull request #359: GEODE-5744: Use fixed width int types.

Posted by "igodwin (GitHub)" <gi...@apache.org>.
Could add braces.

[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pdxcodemonkey commented on pull request #359: GEODE-5744: Use fixed width int types.

Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
Yeah, the 2nd arg with the `type(expr)` cast.  Kinda seems like we should fix all the casts...

[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] igodwin commented on pull request #359: GEODE-5744: Use fixed width int types.

Posted by "igodwin (GitHub)" <gi...@apache.org>.
Again, auto?

[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on issue #359: GEODE-5744: Use fixed width int types.

Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
@igodwin Please re-review.

[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #359: GEODE-5744: Use fixed width int types.

Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
It got rid of the bad cast. Or are you asking about the second argument that is also casted?

[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #359: GEODE-5744: Use fixed width int types.

Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
There is a separate ticket for c-style casts.



[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #359: GEODE-5744: Use fixed width int types.

Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
There is a separate ticket for c-style casts.

The NOLINT will remain so that this method is ABI compatible with the ACE interface it is implementing.

[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] igodwin commented on pull request #359: GEODE-5744: Use fixed width int types.

Posted by "igodwin (GitHub)" <gi...@apache.org>.
Typo: tombstone-gc-threshold
Also on 1309.

[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] igodwin commented on pull request #359: GEODE-5744: Use fixed width int types.

Posted by "igodwin (GitHub)" <gi...@apache.org>.
auto?

[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #359: GEODE-5744: Use fixed width int types.

Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Most of this code was auto fixed using `clang-tidy -fix`. I suppose I can go back and touch up all the fixed lines with cleaner C++.

[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] igodwin commented on pull request #359: GEODE-5744: Use fixed width int types.

Posted by "igodwin (GitHub)" <gi...@apache.org>.
return !(m_caching == "false") - too cryptic?

[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] igodwin commented on pull request #359: GEODE-5744: Use fixed width int types.

Posted by "igodwin (GitHub)" <gi...@apache.org>.
Should this go into develop?

[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #359: GEODE-5744: Use fixed width int types.

Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
I don't understand your question.


[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #359: GEODE-5744: Use fixed width int types.

Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
The NOLINT will remain so that this method is ABI compatible with the ACE interface it is implementing.

[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett closed pull request #359: GEODE-5744: Use fixed width int types.

Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
[ pull request closed by pivotal-jbarrett ]

[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] igodwin commented on pull request #359: GEODE-5744: Use fixed width int types.

Posted by "igodwin (GitHub)" <gi...@apache.org>.
Why this type of cast?

[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #359: GEODE-5744: Use fixed width int types.

Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Not my typo.

[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #359: GEODE-5744: Use fixed width int types.

Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
There is another ticket for `clang-tidy` that addresses those.

[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pdxcodemonkey commented on pull request #359: GEODE-5744: Use fixed width int types.

Posted by "pdxcodemonkey (GitHub)" <gi...@apache.org>.
We were just wondering if the NOLINT annotation needs to be in the mainline sources, or if it's just something you needed to add to work around something and you no longer need it.

[ Full content available at: https://github.com/apache/geode-native/pull/359 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org