[#33] Address Twisted needs

[danuker]

Fixes #33

• Default reviewers configurable per repo (Handle case when reviewers marker is missing #42)
• Use the "Reviewers" feature of GitHub for tracking who still has to review, not the "Assignees". (For "needs-review" marker ask for a review via the "Reviewers" PR metadata. #44)
  • Issues are not yet supported. For issues, must still use the "Assignee" field. But this will be done in Handle issues also, not just PRs #41.
• Do not crash when unable to set assignees (UnprocessableEntity error).
  • The only times to modify assignees: needs-changes and approve-changes.
• Removed old Twisted dependency
• Allow ignoring certain PRs
• Added support for asking review from a team; awaiting team review after all users approved; removing teams when needs-changes

TODO:

• needs-review from a "review" asks for another review (useful for pre-real-review comments on code)
• more flexible matching for please review text
• Fix error Investigate bug #46 (comment)

Not implemented:

• need discussing: approving a PR via comment, as a member of a team. It would be harder (probably another GH API request) to detect if the commenter is member of a team, to check if to remove the team from the team-review-requests. Is it worth it?
• Automated deployment. this will be done in Auto-deployment to Azure Functions #20

reviewers: @adiroiban

The new version was deployed manually (instructions in README.rst). Feel free to test!

[danuker]

Testing attempt to assign @octocat

needs-review

[danuker]

needs-changes

Never mind, the new bot is not deployed yet.

[danuker]

please review
testing

[danuker]

needs-review

testing

[danuker]

please review
testing

[danuker]

please review
testing again

[danuker]

please review :)

Feel free to keep it in review for a week, while we test it extensively.

[adiroiban]

I think that the review request regex should be more relaxed... to allow requesting a review via this "natural language"

this is ready. Please review. Thanks!

---

I now found out about the CODEOWNERS feature.
I think it can be used to automatically request a review from the team, when a PR is created.

But I think that our current implementation is better as you can re-request the review later.
I have not used CODEOWNERS, but I guess it only ask for the team once.
It might re-request is you mark a PR as draft and then back as final

[adiroiban]

CODEOWNER only adds the review first time the PR is created.
it doens't re-add the team on re-review request. Even after marking a PR as draft and then as ready again.

So our hook is still very useful... unfortunately. The goal is to have all these actions supported by Github.

[adiroiban]

It would be nice if 'needs-review' would work for a normal review comment.

See example here twisted/twisted#11629 (review)

The idea is that you leave inline comments and then leave a final comment with the marker.
And the review process is trigerred

[adiroiban]

Let me know when this is deployed.

We also need to allow please review. and please review! ... I guess

Thanks

[danuker]

We also need to allow please review. and please review! ... I guess

The regex I used does not match anything after please review.

So, your examples will match. But so will please review another PR not this one. Is this a problem?

[adiroiban]

So, your examples will match. But so will please review another PR not this one. Is this a problem?

It should not be a problem.
This is what was asked by other Twisted devs.

[danuker]

needs-changes is still case sensitive. I didn't think it'd matter since we should be using reviews anyway.

[danuker]

But Please review! <-- should trigger the change.

I have deployed the last commit (#df0edf13).

Feel free to take more time again for testing before I merge.

[adiroiban]

This is already a branch too old and too big and already in prod.

I think is best to merge it.

If anything needs a fix we should create a separate PR.

Thanks!

[adiroiban]

we will need to see how to re-deploy it for Twisted Org only.
Is ok to share it during development... but for production is best to have separate accounts.

btw... there a are now new personal API tokens with more granular access

[adiroiban]

https://github.blog/changelog/2022-10-18-introducing-fine-grained-personal-access-tokens/

[danuker]

we will need to see how to re-deploy it for Twisted Org only. Is ok to share it during development... but for production is best to have separate accounts.

All right. Should I create a new ProAtria Azure Functions app with the twisted-trac token? Or another, new user, maybe twisted-bot?

btw... there a are now new personal API tokens with more granular access

I saw them, but I can only create them for repos owned by my account.

[adiroiban]

I don't know what is best.

Twisted has its own Azure organization...so is best to host the functions there...
but if you will be the one looking after the code, we can continue to keep it shared with our Chevah org.

I will try to find some time and see what can be done for Twisted.