-
Notifications
You must be signed in to change notification settings - Fork 165
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
#1022 TempFolder #1049
#1022 TempFolder #1049
Conversation
Job #1049 is now in scope, role is |
This pull request #1049 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 @llorllale/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.
@Iprogrammerr Done. Please, check it out.
* closed. | ||
* @since 1.0 | ||
*/ | ||
public final class TempFolder implements Scalar<Path>, Closeable { |
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.
@Iprogrammerr Please, create a new secondary ctor which create a TempFolder
with a random name by default. This name should start with tmp
prefix and more 5 random chars (it sholdn't exceed 8 chars size).
* @param path Relative path to new directory. | ||
* @since 1.0 | ||
*/ | ||
public TempFolder(final String path) { |
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.
@Iprogrammerr This ctor should be final Text path
.
private final Scalar<Path> folder; | ||
|
||
/** | ||
* Ctor. |
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.
@Iprogrammerr Create a new secondary ctor which signature should be TempFolder(final String path)
.
@fabriciofx Done. |
new TextOf(""), | ||
new TextOf("tmp"), | ||
// @checkstyle MagicNumber (1 line) | ||
new RandomText(5, 'a', 'b', 'c') |
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.
@Iprogrammerr RandomText
should cover [a-zA-Z0-9]
.
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.
@fabriciofx How to do that? RandomText does not accept regex. You want me to create ListOf with 62 elements given in a ctor?
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.
@Iprogrammerr I said [a-zA-Z0-9]
just to simplify. The idea is use a...z
, A...Z
and 0...9
. What's the problem with a ctor with 62 elements? Look inside RamdomText
. It has a ctor with more than 62 elements (I just don't say to use this ctor because it has special characters too).
@fabriciofx Refactored. |
@Iprogrammerr This PR seems fine to me, but due the |
@rultor merge |
@fabriciofx Thanks for your request. @llorllale Please confirm this. |
@Iprogrammerr pull from |
@llorllale I don't understand, how pulling is supposed to do that? It will trigger nothing. Didn't you mean merge? |
@Iprogrammerr I just meant to update your PR with |
@llorllale Done. |
Codecov Report
@@ Coverage Diff @@
## master #1049 +/- ##
============================================
+ Coverage 87.58% 87.65% +0.07%
- Complexity 1520 1527 +7
============================================
Files 267 268 +1
Lines 3897 3922 +25
Branches 215 215
============================================
+ Hits 3413 3438 +25
Misses 435 435
Partials 49 49
Continue to review full report at Codecov.
|
@rultor merge |
@llorllale OK, I'll try to merge now. You can check the progress of the merge here |
@llorllale Done! FYI, the full log is here (took me 13min) |
Code review was too long (10 days), architects (@llorllale) were penalized, see §55 |
The job #1049 is now out of scope |
Order was finished: +15 point(s) just awarded to @fabriciofx/z |
Payment to |
As described in #1022. New class for creating temporary directories was created. It is very similar to TempFile, but it can create directories only.