You are here: Foswiki>Tasks Web>Item11624 (20 Feb 2017, GeorgeClark)Edit Attach

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

pencil
Priority: Low
Current State: No Action Required
Released In: n/a
Target Release: n/a
Applies To: Engine
Component: FoswikiMeta, FoswikiStore, CodeRefactoring
Branches: Release01x01
Reported By: SvenDowideit
Waiting For:
Last Change By: GeorgeClark
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

Deferring to 2.0. No work for 2 years now.

-- GeorgeClark - 02 Jun 2014

And removing planned release. If someone needs this or thinks that it's a good idea, please bump the priority.

-- GeorgeClark - 26 Nov 2015

Setting to no action. I keep tripping over this as incomplete work when reviewing open tasks with commits. It's all reverted. Nothing here. Move along.

-- GeorgeClark - 20 Feb 2017
 
Topic revision: r17 - 20 Feb 2017, GeorgeClark
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy