[Zope-Coders] Re: [Zope-Checkins] CVS: Zope/lib/python/ZPublisher - Converters.py:1.14.8.2

Toby Dickenson tdickenson@geminidataloggers.com
Fri, 13 Sep 2002 16:38:25 +0100


On Friday 13 Sep 2002 3:46 pm, Chris Withers wrote:
> Shane Hathaway wrote:
> > 1) Are you sure this was broken?  _updateProperty(), which
> > manage_editProperties() calls, converts the type already.
>
> You're right, this bit of the fix is unnecessary, I'll back it out. I m=
ight
> even optimise _updateProperty while I'm at it, unless anyone can tell m=
e
> why we need "type(value)=3D=3Dtype('')" rather than
> "isinstance(value,StringType)".
>
> I'll await a go-ahead and make these changes.
>
> The bug (which I fixed) was that ZPublisher.Converters didn't map 'mult=
iple
> selection' to the field-to-list convertor. That meant that if you hit t=
he
> edit button in CMF Type Information Objects' Properties tab, you got we=
ird
> an wodnerful errors if implicitly addable was unchecked.
>
> > 2) I don't feel good about this going into the 2_5 branch.  We discus=
sed
> > recently that the Zope release cycle needs to change in order to sque=
eze
> > out the notion of "hotfixes".  Specifically, only security-related
> > bugfixes should go into the 2_5 branch, except when specifically
> > approved by Brian.  That enables us to release a micro revision of Zo=
pe
> > at any time instead of a sysadmin-frightening hotfix, without *any*
> > worries of breaking sites.
>
> Fair enough, I wasn't aware of this change in policy. Can you elaborate=
 on
> this new policy

I dont think we have had a clear policy statement, and you are right that=
 we=20
probably should have one. Here are my thoughts specifically relating to y=
our=20
change:

1. Changes shouldnt go onto a maintenance branch without reaching a conce=
nsus=20
on zope-dev. Even the 'latest cvs' on the maintenance branch should be fr=
ee=20
of suprises.

2. The ChangeLog should include a rationale for the change, not just a=20
statement about what has changed. We need this to give people who are=20
installing the maintenance release confidence that we are taking care of=20
their stability.

3. Things on a maintenance branch should be arranged to minimise the size=
 of=20
the diff. Your patch also tidied up indentation - that just makes the hot=
fix=20
(ahem, maintenance release) seem more intrusive than it really is. It als=
o=20
makes it a fraction harder for anyone wanting to verify for themselves th=
at=20
the release is risk-free before installing it.

4. The maintenance release should address stable installations. We should=
=20
exclude fixes for problems that will only be discovered during developmen=
t.

> > This change seems like it could break sites.
>
> Only if they relied on this obscure and broken behaviour, which I would=
 be
> really suprised to hear of :-S

That reasoning is sufficient to get the change into 2.6, even at this lat=
e=20
stage. To get into a maintenance release we need to know that:
* not including the change will cause problems for a significant number o=
f=20
  stable installation.
* including the fix will not cause any problems for anyone

(All of the above is IMO of course. Comments welcome)



> (The other option is to remember to keep patching Zope until I have a
> compelling reason to move to 2.6.x :-S)

I think this would be the right option for this patch. It is easily done =
if=20
you keep your change in a branch, and write a script to use in place of '=
cvs=20
update' to merge in all your favorite patches.

I cant remember the last time I ran a standard version of Zope. Its no bi=
g=20
deal.