On 04/24/2015 11:47 PM, Jan Heylen wrote:
On Fri, Apr 24, 2015 at 7:21 PM, Mads Kiilerich <m...@kiilerich.com> wrote:
On 04/14/2015 06:03 PM, Jan Heylen wrote:
# HG changeset patch
# User Jan Heylen <heyl...@gmail.com>
# Date 1427526729 -3600
# Sat Mar 28 08:12:09 2015 +0100
# Node ID f26b18234a7731a361f75e7a68ad143fc78218bf
# Parent f48bc03627761dd62f7e7094bb0d322834da336c
changeset: draft comment on changesets
This doesn't include any template changes at all so I doubt that description
is exact?
Changeset is a controller? I'll add "(model)" in the commit message?
I like it when a commit message also summarizes what the changeset does
so I can verify that it really does what it was intended to do ...
especially when we don't have any tests that shows what changes. Ideally
so that someone else based on that description could re-implement pretty
much the same thing. But we should also be pragmatic ...
+ for drafts in lines.values():
+ c.draft_cnt += len(drafts)
This extra iteration is inefficient. Let's hope it doesn't touch unloaded
objects where it would have to touch the db for each iteration.
Unfortunately that is how we do it ...
comment_cnt is also there, Is that not needed neither?
What? I don't get that.
Please also consider adding test coverage for this new functionality.
Yes, I consider this, but I first wanted to get a feeling (and I still
do) on how feasible it is to get this patch series upstream (after
adding tests).
Yes, I'm sure we can get something like this upstreamed. I just want to
make sure we only add stuff that everybody do (or should) consider an
improvement, moves in the right direction and keep reducing technical debt.
/Mads
_______________________________________________
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
