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 message for asking contributors to assign the required reviewer(s) for a PR. #335

Open
seanlip opened this issue Mar 22, 2024 · 9 comments
Labels
enhancement New feature or request good first issue Good for newcomers Impact: Low Work: Low

Comments

@seanlip
Copy link
Member

seanlip commented Mar 22, 2024

Is your feature request related to a problem? Please describe.

When a new PR is opened, Oppiabot leaves a message saying

"Hi @xxx please assign the required reviewer(s) for this PR. Thanks!"

However, for new contributors who don't have "assign" rights, this results in them not being sure what to do.

Describe the solution you'd like

I'd suggest changing the message to:

"Hi @xxx, please assign the required reviewer(s) for this PR. This is necessary for your PR to get reviewed and merged.

Note: If you don't have permissions to modify the "Assignees" section, just leave a comment here saying @{{reviewer_username}} PTAL and I'll help you assign them. Thanks!"

@Flying-Pegasus
Copy link
Contributor

Hey @seanlip is this issue solved by @vojtechjelinek ? If yes then please close the issue and if not then please assign it to me.

@seanlip
Copy link
Member Author

seanlip commented Dec 9, 2024

@Flying-Pegasus I don't think it's been solved. I deassigned @vojtechjelinek.

However, per the guidance at https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#choosing-a-good-first-issue, please provide an explanation of what your PR will do (with names of files you're changing, what you plan to change in each file, etc.), as well as a video showing that the changes work correctly on your local machine. If it looks good, we can assign you to this issue. Thanks!

@Flying-Pegasus
Copy link
Contributor

Flying-Pegasus commented Dec 9, 2024

Proposed: Improve message for asking contributors to assign the required reviewer(s) for a PR.

Files to be Updated

  • checkPullRequestLabels.js .

Proposed Changes

Existing line of code:

const message =
   'Hi @' +
   author +
   ' please assign the required reviewer(s) for this PR. Thanks!';

Updated statement in the code:

const message =
   'Hi @' +
   author +
   ' please assign the required reviewer(s) for this PR.' +
   ' This is necessary for your PR to get reviewed and merged.' +
   'Note: If you do not have permissions to modify the Assignees section, just leave a comment here saying @{{reviewer_username}} PTAL and I will help you assign them. Thanks!' ;

Why These Changes Are Necessary

  • For new contributors who don't have "assign" rights, this will help them what to do.

Next Steps

Hey, @seanlip if this issue is assigned to me, I will:

1.Implement the above changes in the checkPullRequestLabels.js.
2.Submit a pull request for review.

@seanlip
Copy link
Member Author

seanlip commented Dec 9, 2024

@Flying-Pegasus I think you need to do a bit more here. Please read the full issue description carefully including the last paragraph.

@Flying-Pegasus
Copy link
Contributor

Okay you mean I have to add

Note: If you don't have permissions to modify the "Assignees" section, just leave a comment here saying @{{reviewer_username}} PTAL and I'll help you assign them. Thanks!"

This statement as well? @seanlip

@seanlip
Copy link
Member Author

seanlip commented Dec 9, 2024

Yes.

@Flying-Pegasus
Copy link
Contributor

@seanlip I have updated the comment please review it. Thank you.

@seanlip
Copy link
Member Author

seanlip commented Dec 10, 2024

@Flying-Pegasus Could you please test this and demonstrate that your changes are correct (show a screenshot the message that Oppiabot leaves)? This will help you find the improvements that need to be made to your code.

@Flying-Pegasus
Copy link
Contributor

Okay

@seanlip seanlip added Impact: Low Work: Low good first issue Good for newcomers enhancement New feature or request labels Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers Impact: Low Work: Low
Projects
Status: Todo
Development

No branches or pull requests

3 participants