Item11624: reprev comment and number persists in TOPICINFO long after the event.

Priority:	CurrentState:	AppliesTo:	Component:	WaitingFor:
Normal	Waiting for Feedback	Engine		Main.SvenDowideit

resulting in bizzareness like:

META:TOPICINFO{author="sven" comment="reprev" date="1331186181" format="1.1" reprev="8" version="10"}

anyone testing for topicinfo.reprev in an If or query will be stuffed up a bit.

-- SvenDowideit - 08 Mar 2012

First question is, why would anyone be searching for a reprev? The reprev is only relevant when reprev==version, which is an indicator that version was generated by a reprev. Yes, it's noise once version is incremented, but it's also harmless, I think.

Why is this Urgent? I don't see how it can be a release blocker. The following patch probably removes redundant reprev, but I'm not convinced it's worth bothering with.

Index: lib/Foswiki/Store/VC/Store.pm
===================================================================
--- lib/Foswiki/Store/VC/Store.pm   (revision 14255)
+++ lib/Foswiki/Store/VC/Store.pm   (working copy)
@@ -117,6 +117,7 @@
         for my $i (qw(author version date)) {
             $ri->{$i} = $truth->{$i};
         }
+   undef $ri->{reprev};
     }
 
     $gotRev = $version;
Index: lib/Foswiki/Meta.pm
===================================================================
--- lib/Foswiki/Meta.pm   (revision 14255)
+++ lib/Foswiki/Meta.pm   (working copy)
@@ -1865,6 +1865,9 @@
     # (side effect of getRevisionInfo)
     $this->getRevisionInfo();
 
+    # Clear the reprev flag
+    undef $this->get('TOPICINFO')->{reprev};
+
     # Semantics inherited from Cairo. See
     # Foswiki:Codev.BugBeforeSaveHandlerBroken
     if ( $plugins->haveHandlerFor('beforeSaveHandler') ) {
@@ -3552,8 +3555,11 @@
         # when old code (e.g. old plugins) generated the meta.
         $ti->{version} = Foswiki::Store::cleanUpRevID( $ti->{version} );
         $ti->{rev} = $ti->{version};    # not used, maintained for compatibility
-        $ti->{reprev} = Foswiki::Store::cleanUpRevID( $ti->{reprev} )
-          if defined $ti->{reprev};
+   if ( defined $ti->{reprev} ) {
+       $ti->{reprev} = Foswiki::Store::cleanUpRevID( $ti->{reprev} );
+       undef $ti->{reprev} unless $ti->{reprev} &&
+      $ti->{version} &&
+      $ti->{reprev} == $ti->{version};
     }
     else {
 

Marking for Sven's feedback to understand why this is so important.

-- CrawfordCurrie - 09 Mar 2012

mostly, I worry that releasing inconsistent user facing things will lead to our needing to support them in future.

So if someone wants to test for reprev - for eg a UI icon, and then stumbles upon this, they then add the extra workaround that you mention (ie, convert the IF from defined info.reprev to info.reprev = info.version= - or worse, info.comment = 'reprev'=

then later we need to make all serialisations continue to support this - including writing unit tests, docco and other much harder to support mess.

I didn't mean to commit this yet though, as I want to write unit tests for it. - work in progress for 1.1.5.

-- SvenDowideit - 14 Mar 2012

George reverted, and i don't see hwo i can write the unit tests quickly enough, i'm sick.

so defer til 1.2.0 probably.

-- SvenDowideit - 21 Mar 2012

This is clearly not important enough to be worked on for a year, and I can't see that we would block a release because of this, so downgrading it to Normal.

-- CrawfordCurrie - 15 Mar 2013