Priority: Low
Current State: No Action Required
Released In: n/a
Target Release: n/a
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