Re: [PATCH 2/6] lib: Add per-message last modification tracking

[Austin Clements <aclements@csail.mit.edu>]

[-- Attachment #1: Type: text/plain, Size: 1602 bytes --]

Hi David. I haven't had a chance to look back at the original code, but
your follow-up expanded comment agrees with how I remember this code
working.
On Aug 7, 2015 4:41 PM, "David Bremner" <david@tethera.net> wrote:

> Daniel Schoepe <daniel@schoepe.org> writes:
>
>
> > On Fri, 05 Jun 2015 19:28 +0200, David Bremner wrote:
> >> +        /* Prior to NOTMUCH_FEATURE_LAST_MOD, messages did not
> >> +         * track modification revisions.  Give all messages a
> >> +         * revision of 1.
> >> +         */
> >> +        if (new_features & NOTMUCH_FEATURE_LAST_MOD)
> >> +            _notmuch_message_upgrade_last_mod (message);
> >> [..]
> >> +/* Upgrade a message to support NOTMUCH_FEATURE_LAST_MOD.  The caller
> >> + * must call _notmuch_message_sync. */
> >> +void
> >> +_notmuch_message_upgrade_last_mod (notmuch_message_t *message)
> >> +{
> >> +    /* _notmuch_message_sync will update the last modification
> >> +     * revision; we just have to ask it to. */
> >> +    message->modified = TRUE;
> >> +}
> >> +
> >
> > The comment in the first part says that message without LAST_MOD get a
> > revision of 1, but as far as I can tell, _notmuch_message_sync will
> > actually put the next revision number on the message. If this is what's
> > happening, either the comment or the behavior should be changed,
> > depending on what's intended.
>
> I think the behaviour is OK, but you're correct the comment is
> wrong. I'll see if Austin has any comment on that. I guess it would be
> harmless to initialize upgraded messages to some low revision number,
> but I don't see the benefit.
>

[-- Attachment #2: Type: text/html, Size: 2179 bytes --]