[PATCH 1/5] Add log_msg field

Aristeu Rozanski aris at ruivo.org
Sat Mar 21 03:16:54 EST 2009


> I don't see much value in keeping a separate read-only copy of the
> original patch submission.  If maintainers really wanted to "muck
> around" with the patch object, then I doubt they would care about the
> original patch not being stored in a comment object.  It's not
> patchwork's job to function as a pristine backup for the mailing list
> (we have mailing list archives for that).
why then we keep the headers on the Comment object?
If the output in patchwork will be comments/patch, why do we keep Comment
object at all?

> You mention that you want the ability for maintainers to muck around
> with the patch as they see fit.  However, without a log_msg field in the
> Patch class there is no way to modify the log message of the original
> patch submission in the database without violating your "read-only
> comment 0" argument.  We should still have log_msg in the patch object.
I agree with the log message and subject on the patch. sometimes you
want to improve the log message, subject and patch (refresh) contents
with something more meaningful instead of forcing someone to repost.
Consider this situation:
- a patch comes in
- comments follow
- the patch seems to be OK, you decide to include it on your batch for
  the next version
- the patch fails to apply for something almost trivial including a
  function name that changed by one of the patches on the batch
- you need to refresh the patch and update the log contents to reflect
  the new function name
how do you:
- know easily what changed on the refreshed patch?
- know what changed in the description?

I see two options:
1) keep comment 0 around, unchanged, add log_msg on the patch object
   and keep the patch on the contents of the comment 0
2) create a new updated comment with the refreshed patch and insert it
   into patchwork, creating a new patch that will mark the previous as
   obsolete (something to bind two patches together still has to be done,
   tags?)
 
> > Destroying Comment 0 now creates two types of data in patchworks,
> > replies and patches.
> 
> I'm not sure what you mean here.  Don't we have three types of data
> involved as it stands?  Patches, replies, and copies of the patch
> submission message (comment 0)?
I think the model is: Comment = emails, Patch = object created based on a
certain email.

> > 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.
> 
> True, but if this turns out to be a common occurrence then we should 1)
> Improve patchwork's detection, and 2) Implement a real merge/re-attach
> feature to handle such a case automatically.  It's not hard in Python to
> copy the handful of fields to create a new comment object based on the
> patch object.
what if you have to change something, refresh the patch?

> My main gripes with the current method are:
> 
> 1) It's inconsistent to have fields in the Patch class for msgid, name,
> date, mail headers, and patch content, but NOT the log message.
I agree. specially linking the patch back to the comment using a string
(msgid)

-- 
Aristeu




More information about the Patchwork mailing list