From patchwork Tue Feb 8 17:22:35 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Guilherme Salgado X-Patchwork-Id: 82358 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from bilbo.ozlabs.org (localhost [127.0.0.1]) by ozlabs.org (Postfix) with ESMTP id 39E7FB7148 for ; Wed, 9 Feb 2011 04:22:45 +1100 (EST) Received: from mail-gy0-f179.google.com (mail-gy0-f179.google.com [209.85.160.179]) (using TLSv1 with cipher RC4-MD5 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority" (verified OK)) by ozlabs.org (Postfix) with ESMTPS id BAF74B712F for ; Wed, 9 Feb 2011 04:22:43 +1100 (EST) Received: by gyg8 with SMTP id 8so2506176gyg.38 for ; Tue, 08 Feb 2011 09:22:39 -0800 (PST) Received: by 10.151.106.2 with SMTP id i2mr332070ybm.124.1297185759449; Tue, 08 Feb 2011 09:22:39 -0800 (PST) Received: from [192.168.1.103] ([187.126.145.177]) by mx.google.com with ESMTPS id w24sm493706ybk.21.2011.02.08.09.22.37 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 08 Feb 2011 09:22:38 -0800 (PST) Subject: Re: Pull requests with patches From: Guilherme Salgado To: patchwork In-Reply-To: <1297175972.13357.97.camel@feioso> References: <1297175972.13357.97.camel@feioso> Date: Tue, 08 Feb 2011 15:22:35 -0200 Message-ID: <1297185755.13357.106.camel@feioso> Mime-Version: 1.0 X-Mailer: Evolution 2.30.3 X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org 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. Reviewed-by: Dirk Wallenstein --- 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: