#445 - Extracted a new class and modified another to make it reusable. #463
Conversation
|
This pull request #463 is assigned to @Vatavuk/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 a monetary reward for this job |
|
@0crat refuse |
|
This pull request #463 is assigned to @fanifieiev/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 a monetary reward for this job |
| * Text signature of a class method, extracted from XML Skeleton. | ||
| * @since 0.30.9 | ||
| */ | ||
| public final class XmlMethodSignature implements Text { |
There was a problem hiding this comment.
@vzurauskas I think it is better to extend TextEnvelope
There was a problem hiding this comment.
@fanifieiev Yes, looks better now.
| } | ||
|
|
||
| @Override | ||
| public String asString() throws Exception { |
There was a problem hiding this comment.
@vzurauskas What about utilizing some kind of caching here? Like Sticky .
There was a problem hiding this comment.
@fanifieiev It now extends TextEnvelope, so no need for caching anymore.
|
@vzurauskas ping |
|
@fanifieiev/z this job was assigned to you 5days ago. It will be taken away from you soon, unless you close it, see §8. Read this and this, please. |
|
@fanifieiev Done. |
| * @todo #445:30min This method works only for skeletons with a single | ||
| * class because it assigns all methods in the skeleton to the first class | ||
| * it finds. Instead, it should iterate through classes and then through | ||
| * methods of each class. | ||
| */ | ||
| private static List<Node> build(final Skeleton skeleton) throws IOException { |
There was a problem hiding this comment.
@vzurauskas I think this static method is a good candidate to become a class as stated here https://www.yegor256.com/2017/02/07/private-method-is-new-class.html.
So could you please leave a todo to make it a class?
There was a problem hiding this comment.
@fanifieiev Done.
|
@rultor merge |
@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here |
@paulodamaso Done! FYI, the full log is here (took me 14min) |
|
@sereshqua/z please review this job completed by @fanifieiev/z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed |
|
@0crat quality good |
PR for #445
Extracted a new class. Also noticed a possibility to modify another so it could be reused. That solved the issue of a disabled test which I re-enabled and removed the corresponding
todo.