Open alya opened 2 years ago
Merging #216 (36b0fe1) into main (5426a14) will not change coverage. The diff coverage is
n/a
.:exclamation: Current head 36b0fe1 differs from pull request most recent head 403ec2f. Consider uploading reports for the commit 403ec2f to get more accurate results
@@ Coverage Diff @@
## main #216 +/- ##
=======================================
Coverage 64.80% 64.80%
=======================================
Files 24 24
Lines 1770 1770
=======================================
Hits 1147 1147
Misses 623 623
These changes all seem like improvements, thanks for doing this!
I pushed an update addressing all the feedback above.
We may want to hold off on merging the last commit (403ec2f) until we also have a fix to check for double assignments at the right time, as it's expected to make that possibility come up more often.
I merged the first 4 commits as the series ending with 36b0fe1c11342b3892945c330ec1cf7209e5df7c.
I was recently reminded of the text of our new contributor Zulipbot messages, and at this point I don't think it's worth waiting for technical changes before we make some improvements.
@timabbott would you be comfortable with merging the last commit on this PR? Or maybe we can tweak it a bit to specifically address double assignments, e.g.:
"You can contribute by providing feedback on their pull request." -> "You can unassign yourself and contribute by providing feedback on their pull request."
Yeah I think a tweak of that form is probably a reasonable option.
I generally tried to make the language Zulipbot uses clear but more succinct than it currently is, with links to documentation where relevant.
There are no logic changes, and all the variables used were already in the comment.
This PR also adds a checklist for brand new contributors, to encourage claiming the first issue only when ready. As described in the commit message, I think the way this is implemented here (with no logic changes) is not ideal. But we could try it and see how it goes.
I don't know how to test these changes.