You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "hoslo (via GitHub)" <gi...@apache.org> on 2023/12/22 09:23:28 UTC
[PR] refactor(raw): add parse_header_to_str fn [incubator-opendal]
hoslo opened a new pull request, #3804:
URL: https://github.com/apache/incubator-opendal/pull/3804
Part #3793
--
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(raw): add parse_header_to_str fn [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #3804:
URL: https://github.com/apache/incubator-opendal/pull/3804#issuecomment-1868267363
Would you like to refactor APIs like `parse_content_disposition` at the same time? We can do this in next 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.
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(raw): add parse_header_to_str fn [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #3804:
URL: https://github.com/apache/incubator-opendal/pull/3804#discussion_r1435566032
##########
core/src/raw/http_util/header.rs:
##########
@@ -199,6 +200,21 @@ pub fn parse_content_disposition(headers: &HeaderMap) -> Result<Option<&str>> {
}
}
+/// Parse header value to string according to name.
+pub fn parse_header_to_str<K: AsHeaderName>(headers: &HeaderMap, name: K) -> Result<Option<&str>> {
Review Comment:
Would you like to refactor APIs like `parse_content_disposition` at the same time? We can do this in next 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.
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(raw): add parse_header_to_str fn [incubator-opendal]
Posted by "hoslo (via GitHub)" <gi...@apache.org>.
hoslo commented on PR #3804:
URL: https://github.com/apache/incubator-opendal/pull/3804#issuecomment-1868272736
> Would you like to refactor APIs like `parse_content_disposition` at the same time? We can do this in next PR.
What does it mean to change the function signature of `parse_content_disposition` or to replace `parse_content_disposition` with `parse_header_to_str`?
--
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(raw): add parse_header_to_str fn [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #3804:
URL: https://github.com/apache/incubator-opendal/pull/3804#discussion_r1435055608
##########
core/src/raw/http_util/header.rs:
##########
@@ -199,6 +199,21 @@ pub fn parse_content_disposition(headers: &HeaderMap) -> Result<Option<&str>> {
}
}
+/// Parse header value to string.
+pub fn parse_header_to_str(headers: &HeaderMap, name: String) -> Result<Option<&str>> {
Review Comment:
LGTM
--
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(raw): add parse_header_to_str fn [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #3804:
URL: https://github.com/apache/incubator-opendal/pull/3804
--
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(raw): add parse_header_to_str fn [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #3804:
URL: https://github.com/apache/incubator-opendal/pull/3804#issuecomment-1868296565
> What does it mean to change the function signature of parse_content_disposition or to replace parse_content_disposition with parse_header_to_str?
I mean implements parse_content_disposition by parse_header_to_str to remove the duplicated code.
--
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(raw): add parse_header_to_str fn [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #3804:
URL: https://github.com/apache/incubator-opendal/pull/3804#discussion_r1434883909
##########
core/src/raw/http_util/header.rs:
##########
@@ -199,6 +199,21 @@ pub fn parse_content_disposition(headers: &HeaderMap) -> Result<Option<&str>> {
}
}
+/// Parse header value to string.
+pub fn parse_header_to_str(headers: &HeaderMap, name: String) -> Result<Option<&str>> {
Review Comment:
How about accepting a `HeaderName` or `impl Into<HeaderName>`?
--
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(raw): add parse_header_to_str fn [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #3804:
URL: https://github.com/apache/incubator-opendal/pull/3804#discussion_r1435566032
##########
core/src/raw/http_util/header.rs:
##########
@@ -199,6 +200,21 @@ pub fn parse_content_disposition(headers: &HeaderMap) -> Result<Option<&str>> {
}
}
+/// Parse header value to string according to name.
+pub fn parse_header_to_str<K: AsHeaderName>(headers: &HeaderMap, name: K) -> Result<Option<&str>> {
Review Comment:
Would you like to refactor APIs like `parse_content_disposition` at the same time? We can do this in next 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.
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(raw): add parse_header_to_str fn [incubator-opendal]
Posted by "hoslo (via GitHub)" <gi...@apache.org>.
hoslo commented on code in PR #3804:
URL: https://github.com/apache/incubator-opendal/pull/3804#discussion_r1435039010
##########
core/src/raw/http_util/header.rs:
##########
@@ -199,6 +199,21 @@ pub fn parse_content_disposition(headers: &HeaderMap) -> Result<Option<&str>> {
}
}
+/// Parse header value to string.
+pub fn parse_header_to_str(headers: &HeaderMap, name: String) -> Result<Option<&str>> {
Review Comment:
Perhaps we should use the same function signature as `HeaderMap.get`.
```rust
pub fn parse_header_to_str<K: AsHeaderName>(headers: &HeaderMap, name: K) -> Result<Option<&str>> {
```
--
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(raw): add parse_header_to_str fn [incubator-opendal]
Posted by "hoslo (via GitHub)" <gi...@apache.org>.
hoslo commented on code in PR #3804:
URL: https://github.com/apache/incubator-opendal/pull/3804#discussion_r1435519219
##########
core/src/raw/http_util/header.rs:
##########
@@ -199,6 +199,21 @@ pub fn parse_content_disposition(headers: &HeaderMap) -> Result<Option<&str>> {
}
}
+/// Parse header value to string.
+pub fn parse_header_to_str(headers: &HeaderMap, name: String) -> Result<Option<&str>> {
Review Comment:
fixed
--
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