Feature request: Use Cc from commit message to find reviewers

[jherland]

A colleague of mine suggested the following, which I find eminently sensible:

Some people put lines like this at the end of the commit message

Cc: My colleague [email protected]

to indicate that they want their colleague to have a look at the commit in question. git send-email will also interpret this, and add the colleague to the Cc-list of the generate email.

I think it would be good for Critic to do the same, and use Cc-lines from the commit message as an extra source of reviewers to be assigned to the corresponding review.

Now, there is a slight impedance mismatch here, in that Critic assigns reviewers per path, whereas a Cc-annotation is per-commit. Still, I think it would be useful to auto-assign the paths changed by the relevant commit to the mentioned reviewer (even if that would also match other commits where the colleague was not Cc-ed). As always being assigned as a reviewer to some path does not mean that you have to review it, especially not if others are assigned to the same path.

Another potential problem is that the "My colleague [email protected]" annotation does not necessarily map directly to a single Critic user. However, in practice I believe looking up the full name and/or email address would work very well, with very few false positives.

What do you think?

[jensl]

Critic actually assigns changes per path and commit, so assigning all the changes in (only) a certain commit to a user is entirely possible. Whether you want to do that, or assign all changes in the changed files (in all commits,) can be discussed. The problem with assigning only changes in a single commit is how to handle follow-up commits (where the author might forget/skip the CC line.)

Critic has a 1:many mapping between Critic users and "git email addresses" that is used to map commits to users, primarily to determine whether the user is the author and thus shouldn't be reviewed the changes. That mapping could be used here as well, I think.

[jherland]

I think the problem you mention with assigning follow-up commits indeed suggests that we want to apply the assignments to all commits (for those changed files). In any case, if we hit too broadly or narrowly (i.e. assigning a user to too much/little), it is easy for reviewers to adjust their assignments with existing buttons in the web interface.

Using the "git email addresses" seems like a good approach.

BTW: Is the main "Email" address registered with Critic is automatically included in the "Git Emails" as well? From the Home page, it might seem that the "Email" is totally separate from the "Git Emails", and that you would have to manually register your email address in both...

[jensl]

The main email address is not automatically included in the "git emails." I intend for it to be, but have never gotten around to implementing it. In http://critic-review.org/889fd938?review=76 I added a utility function for creating a user and in it I add the initial main email address as a git email address. At some point, that function will be integrated, and used for all user creation, and then it would be fixed.

I should perhaps cherry-pick that commit into its own review; r/76 will probably not be finished in quite a while. Right now, all users are created without git email mappings, which is silly.

[jherland]

I temporarily fixed this in my test install by logging into the database and running:

INSERT INTO usergitemails ( SELECT email, id FROM users );

[jensl]

That's what I did in the system at Opera too, when I first created the usergitemails table. :-)

Created http://critic-review.org/r/105 for some improvements in this area.