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

Improve Zulipbot messages #216

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Improve Zulipbot messages #216

wants to merge 5 commits into from

Conversation

alya
Copy link
Contributor

@alya alya commented Apr 15, 2022

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.

@codecov
Copy link

codecov bot commented Apr 15, 2022

Codecov Report

Merging #216 (36b0fe1) into main (5426a14) will not change coverage.
The diff coverage is n/a.

❗ 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           

@timabbott
Copy link
Member

These changes all seem like improvements, thanks for doing this!

You can reclaim this issue or claim any other issue by commenting `@{username} claim` on that issue.

Thanks for your contributions, and hope to see you again soon!
@{assignee} you have been unassigned from this issue because you have not made any updates for over {total} days. Please feel free to reclaim the issue if you decide to pick up again. Thanks!
Copy link
Member

Choose a reason for hiding this comment

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

This should be either “@{assignee}, you …” with a comma or “@{assignee} You …” with a capital letter. (Similarly for other strings.)

@@ -1,10 +1,9 @@
Welcome to Zulip, @{commenter}! We just sent you an invite to collaborate on this repository at https://github.com/{repoOwner}/{repoName}/invitations. Please accept this invite in order to claim this issue and begin a fun, rewarding experience contributing to Zulip!
Welcome to Zulip, @{commenter}! To get started, please take the following steps **in order** and mark them off when done:
Copy link
Member

Choose a reason for hiding this comment

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

Arbitrary contributors won’t have permission to edit zulipbot’s comment, such as by checking its checkboxes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good to know!

The new language aims to be brief while still providing some motivation for
what's happening.
- The new language is more brief.
- There is an option for the contributor to say that they won't
  be updating their PR.
- Adds an explanation of why linking the PR to the issue is helpful.
- Links to the commit message documentation.
- Shortens the text.
- Adds a link to the contributor guide.
The goal here is to encourage folks not to claim an issue until they
are ready.

We will need to make sure that they don't get assigned the issue if someone
else claims it in the meantime.

We're counting on the contributor not to accept the GitHub invite until
ready, which is not ideal because (a) they might just accept it, and
(b) there's a risk that the invite will expire. So we'd probably be better
off splitting this into two interactions (joining repo; claiming issue).

This also shortens the message if the initial invite hasn't been accepted.
@alya
Copy link
Contributor Author

alya commented Apr 18, 2022

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.

@timabbott
Copy link
Member

I merged the first 4 commits as the series ending with 36b0fe1.

@alya
Copy link
Contributor Author

alya commented Oct 13, 2022

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."

@timabbott
Copy link
Member

Yeah I think a tweak of that form is probably a reasonable option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants