You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "tisonkun (via GitHub)" <gi...@apache.org> on 2024/06/05 00:52:15 UTC
[PR] refactor: centralize range expand logic [opendal]
tisonkun opened a new pull request, #4687:
URL: https://github.com/apache/opendal/pull/4687
Perhaps we can have a better name and better place to hold this logic. Just noticed that we repeat a few time to expand range bounds and they're subtly different.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: centralize range expand logic [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #4687:
URL: https://github.com/apache/opendal/pull/4687#issuecomment-2148736347
I prefer repeating in hand rather than introducing a macro.
The ideal solution will be using `Range<u64>` or `offset, size` directly in `OpRead`. But this could be another breaking change. May be we can implement this change in next release?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: centralize range expand logic [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #4687:
URL: https://github.com/apache/opendal/pull/4687#issuecomment-2148758863
Good to know. Let me make another PR later.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: centralize range expand logic [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun closed pull request #4687: refactor: centralize range expand logic
URL: https://github.com/apache/opendal/pull/4687
--
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.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: centralize range expand logic [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #4687:
URL: https://github.com/apache/opendal/pull/4687#discussion_r1626786184
##########
core/src/lib.rs:
##########
@@ -141,6 +141,24 @@ pub mod layers;
pub mod raw;
pub mod services;
+/// Expand [std::ops::RangeBounds] into (start, end) tuple.
+#[macro_export]
+macro_rules! expand_range {
+ ($range:ident) => {{
+ let start = match $range.start_bound() {
+ std::ops::Bound::Included(n) => Some(n.clone()),
+ std::ops::Bound::Excluded(n) => Some(n.checked_add(1).expect("out of range")),
Review Comment:
`check_add` doesn't belong to a trait and thus we cannot write a function for this usage smoothly ..
--
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.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org