| Message ID | 1296054749-13453-1-git-send-email-halsmit@t-online.de |
|---|---|
| State | Changes Requested |
| Headers | show |
Dear Jeremy, In message <1296054749-13453-1-git-send-email-halsmit@t-online.de> Dirk Wallenstein wrote: > 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@project.tld > > Signed-off-by: Dirk Wallenstein <halsmit@t-online.de> Any chance that this (or something similar) gets activated on patchwork.ozlabs.org ? I'd _really_ like to have such a feature. Best regards, Wolfgang Denk
Hi Wolfgang, > Any chance that this (or something similar) gets activated on > patchwork.ozlabs.org ? > > I'd _really_ like to have such a feature. Yep, that should be fine. I've got a lot of other (ie, non-patchwork) stuff on at the moment, but will take a look at it soon. Cheers, Jeremy
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@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
On Fri, Feb 11, 2011 at 10:16:09AM +0800, Jeremy Kerr wrote: > 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@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. Ups, missed that. > Also, could you add a testcase for these? Let me know if you'd like any help > with that. Just want to say that I'm on it, but I'm having a bit of a cold currently, so it might still take a bit.
Dear Dirk Wallenstein, In message <1296054749-13453-1-git-send-email-halsmit@t-online.de> you wrote: > 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: ... Upon Jeremy Kerr's comment you replied: > > > > > +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. > > Ups, missed that. > > > Also, could you add a testcase for these? Let me know if you'd like any help > > with that. > > Just want to say that I'm on it, but I'm having a bit of a cold > currently, so it might still take a bit. Has anything happened after that? I'd really appreciate to have such a feature. Best regards, Wolfgang Denk
On Tue, Apr 24, 2012 at 08:33:29AM +0200, Wolfgang Denk wrote: > Dear Dirk Wallenstein, > > In message <1296054749-13453-1-git-send-email-halsmit@t-online.de> you wrote: > > 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: > ... > > Upon Jeremy Kerr's comment you replied: > > > > > > > +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. > > > > Ups, missed that. > > > > > Also, could you add a testcase for these? Let me know if you'd like any help > > > with that. > > > > Just want to say that I'm on it, but I'm having a bit of a cold > > currently, so it might still take a bit. > > Has anything happened after that? > > I'd really appreciate to have such a feature. The feature is present. I remember, I was trying to take a step back and create another test base class for the test but a WIP factory by Guilherme was preferred at that time. So, AFAICT the feature is present but a test is missing.
Dear Dirk, In message <20120424111515.GA17273@bottich> you wrote: > > > I'd really appreciate to have such a feature. > > The feature is present. I remember, I was trying to take a step back > and create another test base class for the test but a WIP factory by > Guilherme was preferred at that time. So, AFAICT the feature is present > but a test is missing. By "present" I guess you mean present in the git repo? So the question is when Jeremy finally updates the code on http://patchwork.ozlabs.org - Jeremy, is there any hope we finally can use this great feature? Best regards, Wolfgang Denk
On Thu, Apr 26, 2012 at 02:49:05PM +0200, Wolfgang Denk wrote: > Dear Dirk, > > In message <20120424111515.GA17273@bottich> you wrote: > > > > > I'd really appreciate to have such a feature. > > > > The feature is present. I remember, I was trying to take a step back > > and create another test base class for the test but a WIP factory by > > Guilherme was preferred at that time. So, AFAICT the feature is present > > but a test is missing. > > By "present" I guess you mean present in the git repo? Yes > So the question is when Jeremy finally updates the code on > http://patchwork.ozlabs.org - Jeremy, is there any hope we finally > can use this great feature?
Hi all, >> By "present" I guess you mean present in the git repo? > Yes I'm confused; the patchwork.ozlabs.org site is running the latest patchwork git. However, the latest patchwork git doesn't include this functionality. Last I heard, Dirk was re-rolling the patch for a minor change and to include a test-case. Or did I miss something? Cheers, Jeremy
On Thu, Apr 26, 2012 at 10:15:44PM +0800, Jeremy Kerr wrote: > Hi all, > > >> By "present" I guess you mean present in the git repo? > >Yes > > > I'm confused; the patchwork.ozlabs.org site is running the latest > patchwork git. > > However, the latest patchwork git doesn't include this > functionality. Last I heard, Dirk was re-rolling the patch for a > minor change and to include a test-case. > > Or did I miss something? Now, I'm confused, too. I saw it included (back then) without the minor change and thought it would therefore be okay as is. It is still present in the checkout I just did. I was on my way back to my project anyway and must have forgotten at some point (after drafting the test BC). It doesn't work? I'm afraid I think I don't have a working setup right now.
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' list_id_headers = ['List-ID', 'X-Mailing-List', 'X-list'] whitespace_re = re.compile('\s+') @@ -346,6 +348,24 @@ def clean_content(str): str = sig_re.sub('', str) return str.strip() +def get_state(state_name): + """ Return the state with the given name or the default State """ + if state_name: + try: + return State.objects.get(name__iexact=state_name) + except State.DoesNotExist: + pass + return State.objects.get(name=default_patch_state) + +def get_delegate(delegate_email): + """ Return the delegate with the given email or None """ + if delegate_email: + try: + return User.objects.get(email__iexact=delegate_email) + except User.DoesNotExist: + pass + return None + def parse_mail(mail): # some basic sanity checks @@ -381,6 +401,9 @@ def parse_mail(mail): patch.submitter = author patch.msgid = msgid patch.project = project + patch.state = get_state(mail.get('X-Patchwork-State', '').strip()) + patch.delegate = get_delegate( + mail.get('X-Patchwork-Delegate', '').strip()) try: patch.save() except Exception, ex:
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@project.tld Signed-off-by: Dirk Wallenstein <halsmit@t-online.de> --- The call to strip() was at the wrong place previously. apps/patchwork/bin/parsemail.py | 25 ++++++++++++++++++++++++- 1 files changed, 24 insertions(+), 1 deletions(-)