Oppiabot keeps assigning the wrong reviewers about oppiabot HOT 7 CLOSED

We are assigning based on changelog label and then the person responsible for the project, can do the review or assign the codeowners. The confusion on the above PR is created due to repeated changing of changelog labels.

Also, the automatic assigning of codeowners to PR is something we are targeting for milestone 3. Are there any other cases of the wrong assignment of reviewers?

from oppiabot.

No, this is the only type of case I am aware of. I think also sometimes the changelog labels might not match the actual codeowners, so I was wondering whether it would make sense to bypass that initial reviewer and figure out directly which codeowner would be the "most appropriate reviewer"?

from oppiabot.

What do you mean by "most appropriate reviewer"? I feel it is difficult to define the most appropriate reviewer using the bot and it is better to assign all the codeowners since they all will need to do the review finally.

from oppiabot.

We currently have a review flow where one reviewer looks at the PR in its entirety, and then adds all the codeowners for review of the subparts. So the question I was thinking about is, how do you figure out who that "one reviewer" is?

My suggestion would be that, since each "line of code" is owned by a codeowner, you could add up the lines of code (or, less granularly the number of files) in the PR that are "owned" by each codeowner, categorize these by codeowner, and see which codeowner corresponds to the largest number. This isn't exact but might be a good first guess.

Or, alternatively, you could pick a codeowner at random to take the first-pass review. There may also be other (necessarily approximate) approaches that I haven't thought of.

That said, if you think assigning all the codeowners is better, I don't have a strong objection to that. The only thing to be aware of is that it's possible that this might lead to a somewhat fragmented review because there's a chance that no reviewer would look through the entire PR. But we can still try this and see if this actually happens in practice. I think it's ultimately your call.

(Does this help?)

from oppiabot.

We currently have a review flow where one reviewer looks at the PR in its entirety, and then adds all the codeowners for review of the subparts. So the question I was thinking about is, how do you figure out who that "one reviewer" is?

This is normally the reviewer assigned via the changelog label.

My suggestion would be that, since each "line of code" is owned by a codeowner, you could add up the lines of code (or, less granularly the number of files) in the PR that are "owned" by each codeowner, categorize these by codeowner, and see which codeowner corresponds to the largest number. This isn't exact but might be a good first guess.

Or, alternatively, you could pick a codeowner at random to take the first-pass review. There may also be other (necessarily approximate) approaches that I haven't thought of.

That said, if you think assigning all the codeowners is better, I don't have a strong objection to that. The only thing to be aware of is that it's possible that this might lead to a somewhat fragmented review because there's a chance that no reviewer would look through the entire PR. But we can still try this and see if this actually happens in practice. I think it's ultimately your call.

I think it is feasible to choose the most appropriate reviewer by summing up the lines/files linked with each codeowner but even in that case fragmented review is possible. The most appropriate reviewer will just review the codeowner files and assign other reviewers and there is no guarantee that they will assign the next reviewers (they can miss doing that and only assign the author instead). So, I would prefer the approach of assigning all codeowners and if we see flaws, we can then update it.

from oppiabot.

This is normally the reviewer assigned via the changelog label.

Ah yes, sorry -- for clarification, what I meant (but was unclear about) was "how to figure out which of the codeowners should be the 'one reviewer'". Currently, there's sometimes no overlap between the auto-assigned reviewer and the list of codeowners.

For the rest, I think what you say sounds reasonable. Thanks!

from oppiabot.

This is fixed by #104.

from oppiabot.