You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/05/18 21:45:48 UTC
[GitHub] [arrow-rs] gangliao opened a new pull request #321: Remove super keyword in into_buffer()
gangliao opened a new pull request #321:
URL: https://github.com/apache/arrow-rs/pull/321
# Which issue does this PR close?
#320
# Rationale for this change
clean and elegant
# What changes are included in this PR?
remove super keyword
# Are there any user-facing changes?
no
<!---
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please add the `breaking change` label.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb commented on a change in pull request #321: Remove super keyword in into_buffer()
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #321:
URL: https://github.com/apache/arrow-rs/pull/321#discussion_r634775773
##########
File path: arrow/src/buffer/mutable.rs
##########
@@ -235,7 +235,7 @@ impl MutableBuffer {
}
#[inline]
- pub(super) fn into_buffer(self) -> Buffer {
+ pub fn into_buffer(self) -> Buffer {
Review comment:
doesn't this change effectively mean that `into_buffer()` can now be called from other crates rather than just the parent module?
So in other words this PR adds something to the public API of Arrow.
cc @jorgecarleitao
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb commented on pull request #321: Remove super keyword in into_buffer()
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #321:
URL: https://github.com/apache/arrow-rs/pull/321#issuecomment-864395624
@gangliao are you satisfied with @jorgecarleitao 's suggestion https://github.com/apache/arrow-rs/pull/321#issuecomment-843729878 ? Shall we close this PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb commented on pull request #321: Remove super keyword in into_buffer()
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #321:
URL: https://github.com/apache/arrow-rs/pull/321#issuecomment-864937428
Thank you @gangliao
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] gangliao closed pull request #321: Remove super keyword in into_buffer()
Posted by GitBox <gi...@apache.org>.
gangliao closed pull request #321:
URL: https://github.com/apache/arrow-rs/pull/321
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] gangliao commented on pull request #321: Remove super keyword in into_buffer()
Posted by GitBox <gi...@apache.org>.
gangliao commented on pull request #321:
URL: https://github.com/apache/arrow-rs/pull/321#issuecomment-864415722
Yes. @alamb Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb edited a comment on pull request #321: Remove super keyword in into_buffer()
Posted by GitBox <gi...@apache.org>.
alamb edited a comment on pull request #321:
URL: https://github.com/apache/arrow-rs/pull/321#issuecomment-864395624
@gangliao are you satisfied with @jorgecarleitao 's suggestion https://github.com/apache/arrow-rs/pull/321#issuecomment-843729878 ? Shall we close this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb commented on pull request #321: Remove super keyword in into_buffer()
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #321:
URL: https://github.com/apache/arrow-rs/pull/321#issuecomment-864937428
Thank you @gangliao
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] jorgecarleitao commented on pull request #321: Remove super keyword in into_buffer()
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on pull request #321:
URL: https://github.com/apache/arrow-rs/pull/321#issuecomment-843729878
What do you think about instead making this public, use the `impl From<MutableBuffer> for Buffer`? Seems more idiomatic, and we can just remove this function al-together.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org