-
Notifications
You must be signed in to change notification settings - Fork 202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(#1011) Replaced String.startsWith by cactoos scalar StartsWith #1034
Conversation
This pull request #1034 is assigned to @fabriciofx/z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @paulodamaso/z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewer; there will be no monetary reward for this job |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fanifieiev There are some comments, please take a look
Collections.singletonList( | ||
String.format("HTTP/1.1 %d %s", status, reason) | ||
), | ||
return new Joined<>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fanifieiev PLease se above
req.head() | ||
); | ||
}, | ||
() -> new Filtered<>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fanifieiev Wouldn't be possible to work only with Text
here, without converting them to strings / evaluating it by value
method , to use advantage of the lazy evaluating?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@paulodamaso For now Filtered
accepts Func<X, Boolean>
, so it is not possible to do what you ask for.
Instead the Filtered
has to accept Func<X, Scalar<Boolean>>
, then it will work.
I can create an issue in Cactoos project.
res.head() | ||
); | ||
}, | ||
() -> new Filtered<>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fanifieiev PLease see above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@paulodamaso Please see here
@@ -122,7 +124,9 @@ public void sendError( | |||
); | |||
try { | |||
return new Filtered<>( | |||
hdr -> new Lowered(hdr).asString().startsWith(prefix), | |||
hdr -> new StartsWith( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fanifieiev Here also, please see above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@paulodamaso Please see here
@0crat assign me |
@paulodamaso This pull request #1034 is assigned to @paulodamaso/z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @paulodamaso/z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewer; there will be no monetary reward for this job |
Manual assignment of issues is discouraged, see §19: -5 point(s) just awarded to @paulodamaso/z |
@paulodamaso Please have a look here. |
@fanifieiev Thank you! |
@rultor merge |
@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here |
@fanifieiev @paulodamaso Oops, I failed. You can see the full log here (spent 2hr)
|
@rultor merge |
@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here |
@rultor stop |
@paulodamaso OK, I'll try to stop current task. |
@fanifieiev @paulodamaso Sorry, I failed to stop the previous command, however it has the following result: Oops, I failed. You can see the full log here (spent 3min)
|
Job #1034 is not in the agenda of @paulodamaso/z, can't retrieve data and time of add |
Code review was too long (15 days), architects (@paulodamaso) were penalized, see §55 |
This PR is for issue #1011