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

#704 Enforce failure reasons in tests #744

Closed
wants to merge 2 commits into from

Conversation

neonailol
Copy link
Contributor

@neonailol neonailol commented Mar 20, 2018

Implement #704 as suggested in #588

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

0crat commented Mar 20, 2018

Job #744 is now in scope, role is REV

@0crat
Copy link
Collaborator

0crat commented Mar 20, 2018

This pull request #744 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; this blog post will help you understand what is expected from a code reviewer

@codecov-io
Copy link

codecov-io commented Mar 20, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #744   +/-   ##
=========================================
  Coverage     78.82%   78.82%           
  Complexity     1180     1180           
=========================================
  Files           223      223           
  Lines          3367     3367           
  Branches        191      191           
=========================================
  Hits           2654     2654           
  Misses          666      666           
  Partials         47       47

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 63f3b7f...7625549. Read the comment docs.

@0crat
Copy link
Collaborator

0crat commented Mar 25, 2018

@fabriciofx/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.

@0crat
Copy link
Collaborator

0crat commented Mar 28, 2018

@fabriciofx/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 30, 2018

@fabriciofx/z this job was assigned to you 10days 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 30, 2018

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

@0crat
Copy link
Collaborator

0crat commented Mar 30, 2018

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

@0crat
Copy link
Collaborator

0crat commented Apr 20, 2018

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

@SharplEr
Copy link
Contributor

@neonailol Looks nice but as I understand I can't approve PR which builds with errors. I see only one way to solve it. You can add an empty string for all reasons of failure. After that, we can create an issue for adding the normal reasons. You can also use pdd, it is not hard, just look how people use it in jpeek project.
If you know about some other ways to fix build let me know.

@0crat
Copy link
Collaborator

0crat commented Apr 25, 2018

@SharplEr/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.

@SharplEr
Copy link
Contributor

@llorllale the author doesn't react to my comment. How long should I wait?

@llorllale
Copy link
Contributor

@SharplEr thanks

@llorllale llorllale closed this Apr 28, 2018
@SharplEr
Copy link
Contributor

@llorllale You're welcome

@0crat
Copy link
Collaborator

0crat commented Apr 28, 2018

@elenavolokhova/z please review this job completed by @SharplEr/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 28, 2018
@0crat
Copy link
Collaborator

0crat commented Apr 28, 2018

The job #744 is now out of scope

@0crat
Copy link
Collaborator

0crat commented Apr 28, 2018

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

@0crat
Copy link
Collaborator

0crat commented Apr 28, 2018

The job is not in WBS, won't close the order

@elenavolokhova
Copy link

@0crat quality bad

@0crat
Copy link
Collaborator

0crat commented Apr 29, 2018

Quality is low, no payment, see §31

@0crat
Copy link
Collaborator

0crat commented Apr 29, 2018

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

@neonailol
Copy link
Contributor Author

@SharplEr I think PR with errors is perfectly mergeable, as resolving this errors is not a part of the original task, as yegor said numerous times if 15 minutes not enough for completing a task, and you provide only tests it's perfectly fine

@SharplEr
Copy link
Contributor

@neonailol I think just adding "" can't take a lot of time. And I always thought master should be valid anytime.
@llorllale Am I right about errors?

@llorllale
Copy link
Contributor

@neonailol @SharplEr allowing a faulty build into master at this stage of the cactoos project will introduce too much noise and extra work. We have many contributors - we need a trustworthy build.

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.

6 participants