[PATCH 1/5] Add log_msg field

Don Zickus dzickus at redhat.com
Fri Mar 20 07:22:37 EST 2009


On Thu, Mar 19, 2009 at 01:59:52PM -0500, Nate Case wrote:
> Create a new patch field named 'log_msg' which contains the patch
> commit log message.  Previously this was stored as the first comment
> of the patch.  Storing it explicitly gets rid of the special case
> handling for comment 0 and eliminates the redundant metadata between
> comment 0 and the patch object.  It also has the nice side effect of
> making the number of comments accurately reflect how many replies were
> made to the patch.
> 

<snip>

> +    for patch in patches:
> +        print patch.id, patch.name
> +        if not patch.log_msg:
> +            # Find first comment matching this patch
> +            comment = Comment.objects.filter(patch = patch)[0]
> +            patch.log_msg = comment.content
> +            patch.save()
> +            print "Deleting comment #%d" % comment.id
> +            comment.delete()

Eww. yuck.  I personally believed that patchworks should not only keep all the
comments posted to a mailing list but keep them as read only.  And then
use the Patch class as a workspace (using one of the Comments as a
template).  This would allow maintainers to muck around with the patch as
they see fit for their workflow, but still have the original thread if
they need it.

Destroying Comment 0 now creates two types of data in patchworks, replies
and patches.  What happens when patchwork accidentally labels a reply with
a patch snippet as a Patch?  How do you take that Patch turn it into a
Comment and link it to the appropriate Patch?  It gets kinda messy.

If you treat all the threads as Comments and then Patches as special case
Comments, then moving things around is very simple.  For example
connecting superceded patches together is just a matter of modifying the
patch_ids to point to the new patch and viola you have the full history of
a the patch (well you lose the original patch because patchworks doesn't
have a field in the Comments class for patch_contents).

Just my two cents.

Cheers,
Don



More information about the Patchwork mailing list