Conversation
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| UnusedCode | 1 minor |
| Complexity | 1 critical |
TIP This summary will be updated as you push new changes. Give us feedback
gsprochette
left a comment
gsprochette
left a comment
There was a problem hiding this comment.
Super good PR, thanks for taking care of it. I left a few comments, nothing blocking just questions to make sure the behavior is intentional, I trust you with your final decision :)
| /.github/PULL_REQUEST_TEMPLATE/ @minettekaum @sdiazlor | ||
|
|
||
| # Dependencies and packaging | ||
| /pyproject.toml @gsprochette @begumcig |
There was a problem hiding this comment.
I guess we can also add the pre-commit-hook file, or do you want to dispatch this one manually?
|
|
||
| parts = line.split() | ||
| pattern = parts[0] | ||
| owners = [owner.removeprefix("@") for owner in parts[1:]] |
There was a problem hiding this comment.
should there be if owner.startswith("@") here?
|
|
||
| users_requested, teams_requested = pr.get_review_requests() | ||
| users_requested = list(users_requested) | ||
| if users_requested: |
There was a problem hiding this comment.
should we check that there are at least 2 reviewers? In the case where there is a single one we could automatically assign a second one.
I don't think so, 1 seems enough, just making sure this is on purpose.
There was a problem hiding this comment.
Should we? I thought (especially for simple PRs) we were aiming for 1 reviewer + bugbot?
| login = commit.author.login | ||
| if login in owners: | ||
| latest_owner_matches[login] += file.changes | ||
| break |
There was a problem hiding this comment.
so we're breaking after finding only the single most common author, but we're also looking for 2 reviewers. It seems to me that we would like to get at least the last two commits from each file, which might do +1 to two people or +2 to a single one. Maybe even the last 4 commits?
minettekaum
left a comment
minettekaum
left a comment
There was a problem hiding this comment.
Thanks for creating the PR :D I don't have anything to add to Gaspar's comments
begumcig
left a comment
begumcig
left a comment
There was a problem hiding this comment.
Just one review about the dispatch function, but it's optional so already approved it! Thank you very much, amazing work!!!!
|
|
||
| users_requested, teams_requested = pr.get_review_requests() | ||
| users_requested = list(users_requested) | ||
| if users_requested: |
There was a problem hiding this comment.
Should we? I thought (especially for simple PRs) we were aiming for 1 reviewer + bugbot?
|
|
||
| parts = line.split() | ||
| pattern = parts[0] | ||
| owners = [owner.removeprefix("@") for owner in parts[1:]] |
There was a problem hiding this comment.
I have a more general question about this function. As I understand it, it’s meant to retrieve the fallback owners in case other attempts to find an owner fail. In that case, we’re only interested in the line that starts with *, right?
If so, it seems a bit redundant to extract owners for every line before checking the pattern. We could simplify the logic by first checking whether the pattern is *, and only then collecting the owners.
Something like this might be clearer and avoid unnecessary work:
def get_dispatch_owners(codeowners_lines):
"""Return fallback owners from the catch-all `*` CODEOWNERS rule."""
for line in codeowners_lines:
line = line.split("#", 1)[0].strip()
if not line:
continue
parts = line.split()
if parts[0] == "*":
return [owner.removeprefix("@") for owner in parts[1:]]
return []
Great initiative to smoothen the review process 👏 👏 |
5 participants
Description
New workflow to automatically assign the PRs based on the latest update and codeowners. If no updates, I or Minette will dispatch the reviewsers.
Related Issue
Fixes #(issue number)
Type of Change
Testing
uv run pytest -m "cpu and not slow")For full setup and testing instructions, see the Contributing Guide.
Checklist
Thanks for contributing to Pruna! We're excited to review your work.
New to contributing? Check out our Contributing Guide for everything you need to get started.
First Prune (1-year OSS anniversary)
First Prune marks one year of Pruna’s open-source work. During the initiative window, qualifying merged contributions count toward First Prune. You can earn credits for our performance models via our API.
If you’d like your contribution to count toward First Prune, here’s how it works: