Hi Kristis,
On Sat, Mar 5, 2011 at 06:06, Kristis Makris <[email protected]> wrote:
> Hi Yavor,
>
> > This is in fact what the comment says "# In Bugzilla 2.20
> > AppendComment was moved to Bugzilla::Bug"
> >
> > I can't see what do you mean is incorrect - I suppose you have a good
> > point but I'm missing it?
>
> I'm sorry. It's actually correct here :)
>
>
> I am seeing semantics that are, on a first look, confusing to me. For
> example the variable:
>
> my $VERSION_2_21_0 = "2.21.0";
>
> I don't understand why v2.21.0 is considered.
>
> So far, "up_to_2_22_0" means major = 2 and minor = 22. That does NOT
> mean that the previous version is 2.21.0. It could be 2.21.87 (if such a
> version exists one day). But using 2.21.0 instead means that something
> like 2.21.3 falls through the cracks.
>
> Then, the previous version may not even be 2.21.x. It may just be
> 2.20.5, but instead we already limited the previous version to be
> 2.21.x.
>
This is just the constant - the way it's used is like that:
elsif ( $self->is_version_lt( $VERSION_2_21_0 ) ) {
Which means Less Than but not equal to 2.21.0 - i.e. something has changed
with 2.21.0 anything earlier than 2.21.0 doesn't have the relevant change.
(OK - I could use 2_22_0 instead of 2_21_0 since 2.21.x was unfinal
development series but anyway the change has been firstly introduced
somewhere in 2.21.x).
It's true that it's possible some change to be backported to older series
but 1) this usually doesn't happen since Bugzilla doesn't seem to try to
avoid backporting incompatible changes; 2) Old releases are getting out of
support relatively soon.
>
>
> > ***
> > Yes, set_version_type result is to much extent ignored:
> > - these up_to_*** strings used to cause those very long expressions
> > which I wanted to avoid
>
> I understand this. This is why this patch was written.
>
> The origin of those very long expressions is software updates, which
> cannot be avoided. Also, you did not have to write these expressions
> yourself; they were there.
>
> This patch was created on the basis that changing "is_version_latest" is
> something that could be avoided. It could, but should it? Avoiding it
> lets someone justify not examining whether the new version of the Scmbug
> Bugzilla backend was in fact compatible with the new version of
> Bugzilla.
>
> I argued for this in the past. Ommiting to check does not make something
> automatically safe.
>
> Then, it does not save us from writing long expressions again. e.g. In
> the following lines:
>
> - if ( $self->is_version_up_to_3_1_2() ||
> $self->is_version_up_to_3_4() ||
> $self->is_version_latest() ) {
> + if ( $self->is_version_gte( $VERSION_3_0_0 ) ) {
>
> What happens if lets say v5_0_0 is no longer compatible? We still have
> to come here and rewrite this to:
>
> if ( $self->is_version_gte( $VERSION_3_0_0 ) &&
> $self->is_version_lt( $VERSION_5_0_0 ) )
>
> And if v5_0_3 becomes compatible again, we have to change this again to:
>
> if ( $self->is_version_gte( $VERSION_3_0_0 ) &&
> ($self->is_version_lt( $VERSION_5_0_0 ) ||
> $self->is_version_gte( $VERSION_5_0_3 )))
>
> This patch does not eliminate long expressions, though you may argue
> that you expect they will be less frequent.
>
> New versions lead to having to deal with multiple such states for
> backwards compatibility. I don't see this changing.
>
OK - agreed about all this here:
- yes we still need to make long expressions sometimes (but not so long
IMO)
- and which is more important - when a new change is introduced in place X
- I want to avoid changing all the other places which have nothing to do
with that change X.
- looping around all "long expressions" when new versions are not
introduced doesn't guarantee we're solving all compatibility problems
either. 1) We're wasting time looping through things which haven't changed
2) We're still required to take care about completely new changes (e.g. like
lsearch) - seems better to focus on 2 for me.
>
>
> > - I didn't want to change existing meaning of
> > "$self->{ version_type }" yet.
> > - set_version_type still has it's role to guard against unsupported
> > versions.
> >
> > ***
> > Yes, checks in other bugtrackers are not addressed (yet). I don't
> > think that's a bad thing - would be better to finish with Bugzilla
> > first.
>
> Yes, that would be ok as a first step.
>
> But I don't agree with having this patch merged.
>
Regards,
Yavor
_______________________________________________
scmbug-users mailing list
[email protected]
http://lists.mkgnu.net/cgi-bin/mailman/listinfo/scmbug-users
