| Message ID | 1440440620-25937-38-git-send-email-damien.lespiau@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
> 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. > > v2: Use two new Patch methods to retrieve the commit message and the > other comments (called answers here) (Jeremy Kerr) > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> How about using tabs to separate the patch from the comments/commit message, a lá GitHub?
On Thu, Sep 10, 2015 at 05:17:55PM +0100, Finucane, Stephen wrote: > > 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. > > > > v2: Use two new Patch methods to retrieve the commit message and the > > other comments (called answers here) (Jeremy Kerr) > > > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > > How about using tabs to separate the patch from the comments/commit message, a lá GitHub? That's definitely a possibility, design is all about iterations really and this initial series definitely can do with more iterations.
> On Thu, Sep 10, 2015 at 05:17:55PM +0100, Finucane, Stephen wrote: > > > 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. > > > > > > v2: Use two new Patch methods to retrieve the commit message and the > > > other comments (called answers here) (Jeremy Kerr) > > > > > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > > > > How about using tabs to separate the patch from the comments/commit > message, a lá GitHub? > > That's definitely a possibility, design is all about iterations really > and this initial series definitely can do with more iterations. Good stuff. Parsing the patch to split out the changes to different files (once again, a lá GitHub) would also be a nice touch. See here: https://github.com/naoyukik/CTags/commit/756bcbe63234a559bd0ee66690811589ea88929d Side note: I've some work done on a split view, which I find easier to read than unified view (especially when you've a large number of changed lines). Both of these ideas can wait - I think the tabs idea is a more immediate fix (i.e. for v3 of this series).
On Thu, Sep 10, 2015 at 05:24:47PM +0100, Finucane, Stephen wrote: > > On Thu, Sep 10, 2015 at 05:17:55PM +0100, Finucane, Stephen wrote: > > > > 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. > > > > > > > > v2: Use two new Patch methods to retrieve the commit message and the > > > > other comments (called answers here) (Jeremy Kerr) > > > > > > > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > > > > > > How about using tabs to separate the patch from the comments/commit > > message, a lá GitHub? > > > > That's definitely a possibility, design is all about iterations really > > and this initial series definitely can do with more iterations. > > Good stuff. Parsing the patch to split out the changes to different > files (once again, a lá GitHub) would also be a nice touch. See here: > > https://github.com/naoyukik/CTags/commit/756bcbe63234a559bd0ee66690811589ea88929d > > Side note: I've some work done on a split view, which I find easier to > read than unified view (especially when you've a large number of > changed lines). Both of these ideas can wait - I think the tabs idea > is a more immediate fix (i.e. for v3 of this series). Actually, if given a choice, I'd rather land this as is and then think about the next iteration. The current state is, IMHO, a bit better than the current design. I'd be fairly cautious to not have big series in flight for too long as it means rebasing down the line and makes it unlikely to ever land.
diff --git a/patchwork/models.py b/patchwork/models.py index 4a1a432..cbc8b51 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -18,6 +18,7 @@ # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA from django.db import models +from django.db.models import Q from django.contrib.auth.models import User from django.core.urlresolvers import reverse from django.contrib.sites.models import Site @@ -260,7 +261,17 @@ class Patch(models.Model): def __unicode__(self): return self.name + def commit_message(self): + """Retrieves the commit message""" + return Comment.objects.filter(patch=self, msgid=self.msgid) + + def answers(self): + """Retrieves the answers (ie all comments but the commit message)""" + return Comment.objects.filter(Q(patch=self) & ~Q(msgid=self.msgid)) + def comments(self): + """Retrieves all comments of this patch ie. the commit message and the + answers""" return Comment.objects.filter(patch = self) def _set_tag(self, tag, count): diff --git a/patchwork/templates/patchwork/patch.html b/patchwork/templates/patchwork/patch.html index b222ebe..5a45016 100644 --- a/patchwork/templates/patchwork/patch.html +++ b/patchwork/templates/patchwork/patch.html @@ -177,12 +177,25 @@ function toggle_headers(link_id, headers_id) >{{ patch.pull_url }}</a> {% endif %} +{% for item in patch.commit_message %} +<h2>Commit Message</h2> +<div class="comment"> +<div class="meta">{{ item.submitter|personify:project }} - {{item.date}}</div> +<pre class="content"> +{{ item|commentsyntax }} +</pre> +</div> +{% endfor %} + +{% for item in patch.answers %} +{% if forloop.first %} <h2>Comments</h2> -{% for comment in patch.comments %} +{% endif %} + <div class="comment"> -<div class="meta">{{ comment.submitter|personify:project }} - {{comment.date}}</div> +<div class="meta">{{ item.submitter|personify:project }} - {{item.date}}</div> <pre class="content"> -{{ comment|commentsyntax }} +{{ item|commentsyntax }} </pre> </div> {% endfor %}
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. v2: Use two new Patch methods to retrieve the commit message and the other comments (called answers here) (Jeremy Kerr) Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> --- patchwork/models.py | 11 +++++++++++ patchwork/templates/patchwork/patch.html | 19 ++++++++++++++++--- 2 files changed, 27 insertions(+), 3 deletions(-)