-
Notifications
You must be signed in to change notification settings - Fork 7
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
correcting wording on PR message #149 #153
base: master
Are you sure you want to change the base?
Conversation
@naazy, hoorah! 🎉 It's review time! 👀 I couldn't help but notice that there isn't an To save you time ⏳ I've added the Reviewer as an Assignee and I've added the Any questions, complaints, feedback, contributions? |
@naazy, hoorah! 🎉 It's review time! 👀 I couldn't help but notice that there isn't an To save you time ⏳ I've added the Reviewer as an Assignee and I've added the Any questions, complaints, feedback, contributions? |
1 similar comment
@naazy, hoorah! 🎉 It's review time! 👀 I couldn't help but notice that there isn't an To save you time ⏳ I've added the Reviewer as an Assignee and I've added the Any questions, complaints, feedback, contributions? |
Codecov Report
@@ Coverage Diff @@
## master #153 +/- ##
==========================================
- Coverage 98.32% 96.89% -1.44%
==========================================
Files 23 23
Lines 179 193 +14
==========================================
+ Hits 176 187 +11
- Misses 3 6 +3
Continue to review full report at Codecov.
|
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.
Looks great @naazy 👍
@@ -20,6 +20,9 @@ defmodule Dwylbot.Rules.PR.AwaitingReview do | |||
reviewers = payload["pull_request"]["requested_reviewers"] | |||
|> Enum.map(&(&1["login"])) | |||
|
|||
assignees = get_assignees_login(payload["issue"]["assignees"]) | |||
allReviewersWereAssigned? = Enum.all?(reviewers, fn(x) -> Enum.member?(assignees, x) end) |
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.
If your using x
as a function arg, consider just using function captures instead.
allReviewersWereAssigned? = Enum.all?(reviewers, &Enum.member?(assignees, &1))
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.
Nice one thanks @Shouston3. Will make the change now 😄
Any questions, complaints, feedback, contributions? |
1 similar comment
Any questions, complaints, feedback, contributions? |
Any questions, complaints, feedback, contributions? |
Any questions, complaints, feedback, contributions? |
Any questions, complaints, feedback, contributions? |
2 similar comments
Any questions, complaints, feedback, contributions? |
Any questions, complaints, feedback, contributions? |
Not quite sure why coverage has suddenly dropped 😢 |
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.
@naazy if you look at adding a test for when all reviews are assigned we'll reach 100% coverage
correcting wording on PR message #149