Skip to content
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

#702 Remove temporary files in tests #728

Closed
wants to merge 1 commit into from
Closed

#702 Remove temporary files in tests #728

wants to merge 1 commit into from

Conversation

smyachenkov
Copy link
Contributor

Issue #702.
Replace Files#createTempFile with TemporaryFolder to ensure that all temporary files will be removed after unit tests are complete.

@0crat 0crat added the scope label Mar 8, 2018
@0crat
Copy link
Collaborator

0crat commented Mar 8, 2018

Job #728 is now in scope, role is REV

@0crat
Copy link
Collaborator

0crat commented Mar 8, 2018

This pull request #728 is assigned to @neonailol/z, here is why. The budget is 15 minutes, see §4. Please, read §27 and when you decide to accept the changes, inform @yegor256/z (the architect) right in this ticket. If you decide that this PR should not be accepted ever, also inform the architect.

@codecov-io
Copy link

Codecov Report

Merging #728 into master will decrease coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #728      +/-   ##
============================================
- Coverage     75.79%   75.73%   -0.06%     
+ Complexity     1124     1123       -1     
============================================
  Files           222      222              
  Lines          3342     3342              
  Branches        189      189              
============================================
- Hits           2533     2531       -2     
- Misses          762      764       +2     
  Partials         47       47
Impacted Files Coverage Δ Complexity Δ
src/main/java/org/cactoos/text/TextOf.java 64.17% <0%> (-2.99%) 22% <0%> (-1%)
src/main/java/org/cactoos/io/OutputStreamTo.java 35.48% <0%> (ø) 5% <0%> (ø) ⬇️
src/main/java/org/cactoos/io/WriterTo.java 73.91% <0%> (ø) 8% <0%> (ø) ⬇️
src/main/java/org/cactoos/io/InputStreamOf.java 35.84% <0%> (ø) 10% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41400a5...f7c4ab5. Read the comment docs.

Copy link
Contributor

@neonailol neonailol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rultor good to merge

@smyachenkov
Copy link
Contributor Author

@neonailol rultor was down, can you trigger it once more?

@neonailol
Copy link
Contributor

@rultor good to merge

@rultor
Copy link
Collaborator

rultor commented Mar 12, 2018

@rultor good to merge

@neonailol Thanks for your request. @yegor256 Please confirm this.

@0crat
Copy link
Collaborator

0crat commented Mar 13, 2018

@neonailol/z this job was assigned to you 5 days ago. It will be taken away from you soon, unless you close it, see §8. Read this and this, please.

@neonailol
Copy link
Contributor

@yegor256 Please confirm this.

@0crat
Copy link
Collaborator

0crat commented Mar 16, 2018

@neonailol/z this job was assigned to you 8 days ago. It will be taken away from you soon, unless you close it, see §8. Read this and this, please.

@0crat
Copy link
Collaborator

0crat commented Mar 18, 2018

The user @neonailol/z resigned from #728, please stop working. Reason for job resignation: It is older than 10 days, see §8

@0crat
Copy link
Collaborator

0crat commented Mar 18, 2018

Resigned on delay, see §8: -15 points just awarded to @neonailol/z

@0crat
Copy link
Collaborator

0crat commented Mar 18, 2018

This pull request #728 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 @yegor256/z (the architect) right in this ticket. If you decide that this PR should not be accepted ever, also inform the architect.

@0crat
Copy link
Collaborator

0crat commented Mar 28, 2018

The user @fabriciofx/z resigned from #728, please stop working. Reason for job resignation: It is older than 10 days, see §8

@0crat
Copy link
Collaborator

0crat commented Mar 28, 2018

Resigned on delay, see §8: -15 point(s) just awarded to @fabriciofx/z

@0crat
Copy link
Collaborator

0crat commented Apr 19, 2018

This pull request #728 is assigned to @krzyk/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

@krzyk
Copy link
Contributor

krzyk commented Apr 19, 2018

@flslkxtc please see my comments

@krzyk
Copy link
Contributor

krzyk commented Apr 20, 2018

@llorllale PR author is on vacation, so he won't be able to fix issues here. And because we don't know how long the vacation will take, so I'm rejecting this PR and propose to assign the #702 to someone else (having a PRs and issues opened for long time is bad for the project).

@llorllale
Copy link
Contributor

@krzyk where are your comments for this PR?

* Temporary files generator.
*/
@Rule
public TemporaryFolder folder = new TemporaryFolder();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@flslkxtc Why not final?

* Temporary files generator.
*/
@Rule
public TemporaryFolder folder = new TemporaryFolder();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@flslkxtc Why not final?

* Temporary files generator.
*/
@Rule
public TemporaryFolder folder = new TemporaryFolder();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@flslkxtc Why not final?
And same for all other instances of TemporaryFolder.

@krzyk
Copy link
Contributor

krzyk commented Apr 20, 2018

@llorllale Oops, sorry for that (github changed the file review process since I used it last time). I've submitted the the review comments now. I'll wait for @flslkxtc reply.

@llorllale
Copy link
Contributor

@krzyk I agree to close this PR

@llorllale llorllale closed this Apr 20, 2018
@0crat
Copy link
Collaborator

0crat commented Apr 20, 2018

@elenavolokhova/z please review this job completed by @krzyk/z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed

@0crat 0crat removed the scope label Apr 20, 2018
@0crat
Copy link
Collaborator

0crat commented Apr 20, 2018

The job #728 is now out of scope

@0crat
Copy link
Collaborator

0crat commented Apr 20, 2018

Payment to ARC for a closed pull request, as in §28: +10 point(s) just awarded to @llorllale/z

@elenavolokhova
Copy link

@0crat quality acceptable

@0crat
Copy link
Collaborator

0crat commented Apr 20, 2018

Order was finished, quality is "acceptable": +15 point(s) just awarded to @krzyk/z

@0crat
Copy link
Collaborator

0crat commented Apr 20, 2018

Quality review completed: +8 point(s) just awarded to @elenavolokhova/z

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants