[PATCH v2] Recognize mail headers for delegate and state

Jeremy Kerr jk at ozlabs.org
Fri Feb 11 13:16:09 EST 2011


Hi Dirk,

> Introduce two new Patchwork mail headers that determine the initial
> state and delegate of a patch.  They take a state name as displayed in
> Patchwork and the email address of the wanted delegate.  An example:
> 
> X-Patchwork-State: Changes Requested
> X-Patchwork-Delegate: maintainer at project.tld

Looks good, except for:

> diff --git a/apps/patchwork/bin/parsemail.py
> b/apps/patchwork/bin/parsemail.py index 1b73169..2a4df38 100755
> --- a/apps/patchwork/bin/parsemail.py
> +++ b/apps/patchwork/bin/parsemail.py
> @@ -34,8 +34,10 @@ except ImportError:
>      from email.Utils import parsedate_tz, mktime_tz
> 
>  from patchwork.parser import parse_patch
> -from patchwork.models import Patch, Project, Person, Comment
> +from patchwork.models import Patch, Project, Person, Comment, State
> +from django.contrib.auth.models import User
> 
> +default_patch_state = 'New'

We're duplicating the default-state logic provided in Patch.save() here, which 
uses the database for a lookup (there may not be a 'New' state). It would be 
better to leave the state un-set in this case, rather than selecting a 
default, then falling back to the default provided in the save() method.

Also, could you add a testcase for these? Let me know if you'd like any help 
with that.

Cheers,


Jeremy


More information about the Patchwork mailing list