Pull requests with patches

[Guilherme Salgado]

On Tue, 2011-02-08 at 12:39 -0200, Guilherme Salgado wrote:
> Hi,
> 
> I was looking at how patchwork deals with pull requests and noticed that
> when it parses a message containing both a pull request and a patch, it
> will create two Patch() instances (one with the patch as its content and
> the other without content but with a pull_url):
> 
>     if patchbuf:
>         mail_headers(mail)
>         name = clean_subject(mail.get('Subject'), [project.linkname])
>         patch = Patch(name = name, content = patchbuf,
>                     date = mail_date(mail), headers = mail_headers(mail))
> 
>     if pullurl:
>         name = clean_subject(mail.get('Subject'), [project.linkname])
>         patch = Patch(name = name, pull_url = pullurl,
>                     date = mail_date(mail), headers = mail_headers(mail))
> 
> However, these are not automatically inserted in the DB, so only the
> second ends up being saved.
> 
> I think it would make sense to create a single Patch() instance in this
> case with both a pull_url and the patch content (although it may be
> necessary to change the template to render such a patch correctly). Is
> there anything that would prevent that from working?

The existing template is able to handle a patch containing both a
pull_url and a non-empty content, and the small change below will make
sure the diff is never thrown away.  If there's interest in this change
I'd be happy to write a test case for it.


---
 apps/patchwork/bin/parsemail.py |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/apps/patchwork/bin/parsemail.py b/apps/patchwork/bin/parsemail.py
index 700cb6f..9f3561c 100755
--- a/apps/patchwork/bin/parsemail.py
+++ b/apps/patchwork/bin/parsemail.py
@@ -183,16 +183,10 @@ def find_content(project, mail):
     patch = None
     comment = None
 
-    if patchbuf:
-        mail_headers(mail)
+    if pullurl or patchbuf:
         name = clean_subject(mail.get('Subject'), [project.linkname])
-        patch = Patch(name = name, content = patchbuf,
-                    date = mail_date(mail), headers = mail_headers(mail))
-
-    if pullurl:
-        name = clean_subject(mail.get('Subject'), [project.linkname])
-        patch = Patch(name = name, pull_url = pullurl,
-                    date = mail_date(mail), headers = mail_headers(mail))
+        patch = Patch(name = name, pull_url = pullurl, content = patchbuf,
+                      date = mail_date(mail), headers = mail_headers(mail))
 
     if commentbuf:
         if patch:
-- 
1.7.1


-- 
Guilherme Salgado <https://launchpad.net/~salgado>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/patchwork/attachments/20110208/aca895fb/attachment.pgp>