You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2016/11/10 21:49:16 UTC
[kudu-CR] macOS: fix shared ptr.h ifdef
Hello Adar Dembo,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/5037
to review the following change.
Change subject: macOS: fix shared_ptr.h ifdef
......................................................................
macOS: fix shared_ptr.h ifdef
Despite being libc++, the macOS system C++ library does not define
_LIBCPP_VERSION.
Change-Id: Iff56316ea55244235f3a44df8e0c3f53da1b35c6
---
M src/kudu/client/shared_ptr.h
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/5037/1
--
To view, visit http://gerrit.cloudera.org:8080/5037
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iff56316ea55244235f3a44df8e0c3f53da1b35c6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
[kudu-CR] macOS: fix shared ptr.h includes
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: macOS: fix shared_ptr.h includes
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/5037
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iff56316ea55244235f3a44df8e0c3f53da1b35c6
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] macOS: fix shared ptr.h includes
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged.
Change subject: macOS: fix shared_ptr.h includes
......................................................................
macOS: fix shared_ptr.h includes
_LIBCPP_VERSION was not being defined appropriately in
client_samples-test due to header include order.
Change-Id: Iff56316ea55244235f3a44df8e0c3f53da1b35c6
Reviewed-on: http://gerrit.cloudera.org:8080/5037
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/client/shared_ptr.h
1 file changed, 5 insertions(+), 0 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/5037
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iff56316ea55244235f3a44df8e0c3f53da1b35c6
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] macOS: fix shared ptr.h includes
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: macOS: fix shared_ptr.h includes
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5037/2/src/kudu/client/shared_ptr.h
File src/kudu/client/shared_ptr.h:
PS2, Line 37: _LIBCPP_VERSION
> Nit: _LIBCPP_VERSION
Done
--
To view, visit http://gerrit.cloudera.org:8080/5037
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iff56316ea55244235f3a44df8e0c3f53da1b35c6
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] macOS: fix shared ptr.h includes
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: macOS: fix shared_ptr.h includes
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5037/2/src/kudu/client/shared_ptr.h
File src/kudu/client/shared_ptr.h:
PS2, Line 37: _LIBCPP_VERSIONS
Nit: _LIBCPP_VERSION
--
To view, visit http://gerrit.cloudera.org:8080/5037
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iff56316ea55244235f3a44df8e0c3f53da1b35c6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] macOS: fix shared ptr.h ifdef
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: macOS: fix shared_ptr.h ifdef
......................................................................
Patch Set 1:
Looks good, but on Slack you said Kudu largely builds on macOS even without this fix (only the build in client_samples-test fails). Can you figure out why that is?
I'd like to understand that before merging this, in case something else is going on.
--
To view, visit http://gerrit.cloudera.org:8080/5037
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iff56316ea55244235f3a44df8e0c3f53da1b35c6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] macOS: fix shared ptr.h includes
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5037
to look at the new patch set (#3).
Change subject: macOS: fix shared_ptr.h includes
......................................................................
macOS: fix shared_ptr.h includes
_LIBCPP_VERSION was not being defined appropriately in
client_samples-test due to header include order.
Change-Id: Iff56316ea55244235f3a44df8e0c3f53da1b35c6
---
M src/kudu/client/shared_ptr.h
1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/5037/3
--
To view, visit http://gerrit.cloudera.org:8080/5037
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iff56316ea55244235f3a44df8e0c3f53da1b35c6
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] macOS: fix shared ptr.h includes
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5037
to look at the new patch set (#2).
Change subject: macOS: fix shared_ptr.h includes
......................................................................
macOS: fix shared_ptr.h includes
_LIBCPP_VERSION was not being defined appropriately in
client_samples-test due to header include order.
Change-Id: Iff56316ea55244235f3a44df8e0c3f53da1b35c6
---
M src/kudu/client/shared_ptr.h
1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/5037/2
--
To view, visit http://gerrit.cloudera.org:8080/5037
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iff56316ea55244235f3a44df8e0c3f53da1b35c6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins