| Message ID | 1415473744-31531-41-git-send-email-damien.lespiau@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi Damien, > All 'Comments' are stored the same way in the db, but I believe it's > worth making the distinction between introducing what the patch does and > eventual review comments. > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > --- > apps/patchwork/models.py | 11 ++++++++++- > templates/patchwork/patch.html | 18 +++++++++++++++++- > 2 files changed, 27 insertions(+), 2 deletions(-) > > diff --git a/apps/patchwork/models.py b/apps/patchwork/models.py > index 4bed9b8..b9c7794 100644 > --- a/apps/patchwork/models.py > +++ b/apps/patchwork/models.py > @@ -188,7 +188,16 @@ class Patch(models.Model): > return self.name > > def comments(self): > - return Comment.objects.filter(patch = self) > + comments = Comment.objects.filter(patch = self) > + # len() will trigger a queryset evaluation. That's what we want as > + # we need the full list of comments. That the list is then sliced into > + # (commit message, list of followup comments) won't hit the db twice. > + n_comments = len(comments) > + if n_comments > 2: > + return (comments[0], comments[1:]) > + if n_comments == 1: > + return (comments[0], ()) > + return (None, ()) This doesn't guarantee that you'll pick the correct comment for the commit message - you need to find the comment that has the same message-id as the patch. Also, could we have this as a separate function? I'd like to Patch.comments() for the complete set as a single list. Your template might be a bit cleaner if have separate functions for commit-message-comments and follow-up-comments too. Cheers, Jeremy
diff --git a/apps/patchwork/models.py b/apps/patchwork/models.py index 4bed9b8..b9c7794 100644 --- a/apps/patchwork/models.py +++ b/apps/patchwork/models.py @@ -188,7 +188,16 @@ class Patch(models.Model): return self.name def comments(self): - return Comment.objects.filter(patch = self) + comments = Comment.objects.filter(patch = self) + # len() will trigger a queryset evaluation. That's what we want as + # we need the full list of comments. That the list is then sliced into + # (commit message, list of followup comments) won't hit the db twice. + n_comments = len(comments) + if n_comments > 2: + return (comments[0], comments[1:]) + if n_comments == 1: + return (comments[0], ()) + return (None, ()) def save(self): try: diff --git a/templates/patchwork/patch.html b/templates/patchwork/patch.html index 6540060..618f08e 100644 --- a/templates/patchwork/patch.html +++ b/templates/patchwork/patch.html @@ -177,8 +177,22 @@ function toggle_headers(link_id, headers_id) >{{ patch.pull_url }}</a> {% endif %} +{% for item in patch.comments %} + +{% if forloop.first %} +{% if item %} +<h2>Commit Message</h2> +<div class="comment"> +<div class="meta">{{ item.submitter|personify }} - {{item.date}}</div> +<pre class="content"> +{{ item|commentsyntax }} +</pre> +</div> +{% endif %} +{% elif item %} <h2>Comments</h2> -{% for comment in patch.comments %} + +{% for comment in item %} <div class="comment"> <div class="meta">{{ comment.submitter|personify }} - {{comment.date}}</div> <pre class="content"> @@ -186,6 +200,8 @@ function toggle_headers(link_id, headers_id) </pre> </div> {% endfor %} +{% endif %} +{% endfor %} {% if patch.content %} <h2>Patch</h2>
All 'Comments' are stored the same way in the db, but I believe it's worth making the distinction between introducing what the patch does and eventual review comments. Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> --- apps/patchwork/models.py | 11 ++++++++++- templates/patchwork/patch.html | 18 +++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-)