[RFC] Store patch authors in the new Patch.author field

Jeremy Kerr jk at ozlabs.org
Fri Apr 29 12:10:15 EST 2011


Hi Guilherme,

> When the first line of the email message that contains a patch starts with
> 'From:' and contain an email address, we use that address as the author;
> otherwise we use the same as the submitter.
> 
> Currently this information is not shown anywhere but it could be easily
> exposed in the UI and or over XML-RPC. I'd be happy to do that if there are no
> objections to this patch.

I'd be happy to apply this; the tricky bit will be how to expose this in
the UI, but we can work that out separately.

One question though: should we allow From: to be on any line of a patch,
rather than the first? Consider something like:

 http://patchwork.ozlabs.org/patch/92959/

For ubuntu-kernel, when proposing an update patch, we forward the entire
commit (possibly with comments before it, like why we're proposing the
patch for that ubuntu kernel version). In this case, it'd be correct to
parse the *last* From: line before the patch itself.

A couple of comments:

> +def find_author(comment):
> +    """Return a Person entry for the author specified in the given comment.
> +
> +    The author of a patch is sometimes not the person who's submitting it, and
> +    in these cases the first line of the email will specify the author, using
> +    the following format:
> +
> +        From: Somebody <somebody at somewhere.tld>
> +
> +    When the first line starts with 'From:' and contain an email address, we
> +    return a Person entry representing that email address, creating a new one
> +    if necessary.  If the first line doesn't start with 'From:' or it doesn't
> +    contain an email address, we return None.
> +    """
> +    if not comment:
> +        return None
> +    first_line = comment.content.split('\n', 1)[0]
> +    if first_line.startswith('From:'):
> +        emails = extract_email_addresses(first_line)
> +        if len(emails) == 1:
> +            email = emails[0]
> +            try:
> +                person = Person.objects.get(email__iexact=email)
> +            except Person.DoesNotExist:
> +                name = email.split('@', 1)[0]

hm, we shouldn't be using the username part of the email address here,
instead look for something in the format:

 "Firstname Lastname <me at example.com>"

and rather than re-implementing this, It'd be best to abstract
find_author (which already handles different email formats), and use it
in both author and submitter parsing.

> +                person = Person(name=name, email=email)
> +                person.save()
> +            return person
> +        elif len(emails) == 0:
> +            return None
> +        else:
> +            raise AssertionError(
> +                'This patch has more than one author: %s.' % first_line)

I'd prefer we just pick one here, rather than aborting the parse

Cheers,


Jeremy



More information about the Patchwork mailing list