-
Notifications
You must be signed in to change notification settings - Fork 5
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
#214 new attributes failed_builds
and succeeded_builds
for pull-was-merged
#296
Conversation
@Yegorov @tank-bohr please review |
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.
@Suban05 Look good, but have some question.
@@ -641,6 +636,14 @@ def test_pull_request_event_with_comments | |||
assert_equal(1, f.comments_resolved) | |||
end | |||
|
|||
def test_count_numbers_of_workflow_builds | |||
fb = Factbase.new | |||
load_it('github-events', fb, Judges::Options.new({ 'repositories' => 'foo/foo', 'testing' => true })) |
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.
@Suban05 Why not use web mock for this cases? Testing mode used for run yml files, for example - https://github.com/zerocracy/judges-action/blob/master/judges/github-events/quick-scan.yml
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.
@Yegorov good question. For the pull-was-merged
event, we use Fbe.github_graph
. Maybe I don't understand something, but it's very difficult to mock this thing by WebMock
. And I use a fake client for Fbe.github_graph
. However, there is an options
argument in both clients (Fbe.octo
and Fbe.github_graph
), and when I set testing
globally, Fbe.octo
also becomes a fake client. That's why I use fake clients
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.
👍
@yegor256 please check |
@Suban05 can't merge, there are git conflicts, please resolve them |
@yegor256 done |
@Suban05 thanks! |
@tank-bohr Hey there! 👋 Great job on the review! You've snagged +4 points for this one. Here's the breakdown: +8 as the base, -10 because we only saw 2 comments (we love detailed feedback!), and +6 to keep you above the minimum. Remember, more comments could boost your score next time - up to 8 points for chatty reviews! Your running balance is now at +136. Keep up the good work and happy coding! 💻🚀 |
@Suban05 Great job on your contribution! 🎉 You've earned +21 points: +16 base, +5 for 53 hits-of-code. No deductions for excessive code or lack of reviews. Your balance is now +456. Keep up the good work and remember, quality matters in our review process. Looking forward to your next contribution! |
@yegor256 merge zerocracy/fbe#77 PR before
Closes #214