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

(#1039) Replaced @Rule with Throws matcher #1100

Merged
merged 1 commit into from
Apr 15, 2019
Merged

(#1039) Replaced @Rule with Throws matcher #1100

merged 1 commit into from
Apr 15, 2019

Conversation

atapin
Copy link
Contributor

@atapin atapin commented Apr 8, 2019

It updates the TextTest by using Throws matcher instead of @Rule-marked exception check.

@0crat 0crat added the scope label Apr 8, 2019
@0crat
Copy link
Collaborator

0crat commented Apr 8, 2019

Job #1100 is now in scope, role is REV

@codecov-io
Copy link

codecov-io commented Apr 8, 2019

Codecov Report

Merging #1100 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##             master   #1100   +/-   ##
========================================
  Coverage      88.6%   88.6%           
  Complexity     1576    1576           
========================================
  Files           275     275           
  Lines          3931    3931           
  Branches        214     214           
========================================
  Hits           3483    3483           
  Misses          396     396           
  Partials         52      52

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 a760506...38e0c43. Read the comment docs.

@0crat
Copy link
Collaborator

0crat commented Apr 8, 2019

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

Copy link
Collaborator

@victornoel victornoel left a comment

Choose a reason for hiding this comment

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

@atapin I've left you a "few" comments (like a hundred time the same one ;)

src/test/java/org/cactoos/ScalarTest.java Outdated Show resolved Hide resolved
src/test/java/org/cactoos/ScalarTest.java Outdated Show resolved Hide resolved
src/test/java/org/cactoos/TextTest.java Outdated Show resolved Hide resolved
src/test/java/org/cactoos/TextTest.java Outdated Show resolved Hide resolved
src/test/java/org/cactoos/collection/NoNullsTest.java Outdated Show resolved Hide resolved
src/test/java/org/cactoos/map/MapEnvelopeTest.java Outdated Show resolved Hide resolved
src/test/java/org/cactoos/map/MapEnvelopeTest.java Outdated Show resolved Hide resolved
src/test/java/org/cactoos/map/MapEnvelopeTest.java Outdated Show resolved Hide resolved
src/test/java/org/cactoos/text/StrictTest.java Outdated Show resolved Hide resolved
src/test/java/org/cactoos/text/StrictTest.java Outdated Show resolved Hide resolved
@victornoel
Copy link
Collaborator

@atapin thanks

@llorllale seems good to merge. Not sure why appveyor fails, it is not related to this PR apparently…

@llorllale
Copy link
Contributor

@atapin remove github keywords from the PR description (see our PR template)

@atapin
Copy link
Contributor Author

atapin commented Apr 14, 2019

@llorllale done

@llorllale
Copy link
Contributor

@atapin there are merge conflicts

@atapin
Copy link
Contributor Author

atapin commented Apr 14, 2019

@llorllale fixed

@llorllale
Copy link
Contributor

@atapin just one more minor comment :)

@atapin
Copy link
Contributor Author

atapin commented Apr 15, 2019

@llorllale resolved

@llorllale
Copy link
Contributor

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Apr 15, 2019

@rultor merge

@llorllale OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit 38e0c43 into yegor256:master Apr 15, 2019
@rultor
Copy link
Collaborator

rultor commented Apr 15, 2019

@rultor merge

@llorllale Done! FYI, the full log is here (took me 13min)

@0crat
Copy link
Collaborator

0crat commented Apr 15, 2019

@sereshqua/z please review this job completed by @victornoel/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 15, 2019
@0crat
Copy link
Collaborator

0crat commented Apr 15, 2019

Job #1100 is not in the agenda of @victornoel/z, can't inspect

@0crat
Copy link
Collaborator

0crat commented Apr 15, 2019

The job #1100 is now out of scope

@0crat
Copy link
Collaborator

0crat commented Apr 15, 2019

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

@sereshqua
Copy link

@0crat quality good

@0crat
Copy link
Collaborator

0crat commented Apr 16, 2019

Quality review completed: +4 point(s) just awarded to @sereshqua/z

@0crat
Copy link
Collaborator

0crat commented Apr 16, 2019

Order was finished, quality is "good": +20 point(s) just awarded to @victornoel/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.

7 participants