Re: [Zope3-dev] Cleaning up the widget mess a little bit: bug or feature
On Dec 19, 2006, at 2:34 AM, Christian Theune wrote: Hi again, Gary Poster wrote: I don't have a very strong feeling about it, but lean towards bug fix. It didn't break any of our code (or at least any of our tests :-) ) so it seems safe from my perspective. I was trying to apply the patch to the 3.3 branch and noticed that the patch isn't compatible, as it requires a restructuring that happened on the trunk a while ago. This refactoring (r70331) introduces a very small feature, but the broken behaviour (trying to put anything into _toFormValue) exists in the old variant as well. It's a bit murky, since 70331 only changes internal APIs, but unfortunately the widget subclass API is effectively public in IMO. I don't think 70331 is ok to push back, unfortunately. A shame, but then the easiest thing to do is treat 71548 as a feature too; the other option would be to revise the fix in 71548 for pre-70331. Gary ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
Re: [Zope3-dev] Cleaning up the widget mess a little bit: bug or feature
Hi, Gary Poster wrote: On Dec 19, 2006, at 2:34 AM, Christian Theune wrote: Hi again, Gary Poster wrote: I don't have a very strong feeling about it, but lean towards bug fix. It didn't break any of our code (or at least any of our tests :-) ) so it seems safe from my perspective. I was trying to apply the patch to the 3.3 branch and noticed that the patch isn't compatible, as it requires a restructuring that happened on the trunk a while ago. This refactoring (r70331) introduces a very small feature, but the broken behaviour (trying to put anything into _toFormValue) exists in the old variant as well. It's a bit murky, since 70331 only changes internal APIs, but unfortunately the widget subclass API is effectively public in IMO. I don't think 70331 is ok to push back, unfortunately. A shame, but then the easiest thing to do is treat 71548 as a feature too; the other option would be to revise the fix in 71548 for pre-70331. I've had the same feeling initially. I'm looking at 70331 again and don't think it's too bad. Agreed, the potential for an incompatibility is there, because some client code might have used _getCurrentValue already in a subclass. (But then again, we would face the same problem when introducing _getCurrentValue in general.) The interface for _getFormValue remains stable and the code didn't change. The only thing that changed is that the responsibility of retrieving the value and converting it to it's form representation was split over two methods instead of a single. Christian -- gocept gmbh co. kg - forsterstraße 29 - 06112 halle/saale - germany www.gocept.com - [EMAIL PROTECTED] - phone +49 345 122 9889 7 - fax +49 345 122 9889 1 - zope and plone consulting and development signature.asc Description: OpenPGP digital signature ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
Re: [Zope3-dev] Cleaning up the widget mess a little bit: bug or feature
On Dec 20, 2006, at 7:15 AM, Christian Theune wrote: Hi, Gary Poster wrote: On Dec 19, 2006, at 2:34 AM, Christian Theune wrote: Hi again, Gary Poster wrote: I don't have a very strong feeling about it, but lean towards bug fix. It didn't break any of our code (or at least any of our tests :-) ) so it seems safe from my perspective. I was trying to apply the patch to the 3.3 branch and noticed that the patch isn't compatible, as it requires a restructuring that happened on the trunk a while ago. This refactoring (r70331) introduces a very small feature, but the broken behaviour (trying to put anything into _toFormValue) exists in the old variant as well. It's a bit murky, since 70331 only changes internal APIs, but unfortunately the widget subclass API is effectively public in IMO. I don't think 70331 is ok to push back, unfortunately. A shame, but then the easiest thing to do is treat 71548 as a feature too; the other option would be to revise the fix in 71548 for pre-70331. I've had the same feeling initially. I'm looking at 70331 again and don't think it's too bad. Agreed, the potential for an incompatibility is there, because some client code might have used _getCurrentValue already in a subclass. (But then again, we would face the same problem when introducing _getCurrentValue in general.) The interface for _getFormValue remains stable and the code didn't change. The only thing that changed is that the responsibility of retrieving the value and converting it to it's form representation was split over two methods instead of a single. Hm, ok. That does sound more workable than the skim of the diff appeared. (FWIW, we also have the internal evidence of backwards compatibility that our private and public packages didn't need to be adjusted for this change.) I'll +1 this. :-) Thanks, Christian! Gary ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
Re: [Zope3-dev] Cleaning up the widget mess a little bit: bug or feature
Christian Theune wrote: I'd both apply r70331 and r71548 to 3.3 and 3.2 then. JFTR: I've backported the small restructuring and the bugfix in r71612 (3.2) and r71611 (3.3) Christian -- gocept gmbh co. kg - forsterstraße 29 - 06112 halle/saale - germany www.gocept.com - [EMAIL PROTECTED] - phone +49 345 122 9889 7 - fax +49 345 122 9889 1 - zope and plone consulting and development signature.asc Description: OpenPGP digital signature ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
Re: [Zope3-dev] Cleaning up the widget mess a little bit: bug or feature
On Dec 14, 2006, at 7:55 AM, Christian Theune wrote: Hi, ... Zagy and I fixed this issue, by making the methods _getCurrentValue and _getFormValue use a common method to retrieve the input value and handle the case of converting to the form value cleanly. However, we are not sure, whether this should be considered a bug or a feature and would like to discuss whether this should be back ported. You can find the change set in r71548. (We documented the change in the 'Bug fixes' section of the CHANGES.txt for now.) I don't have a very strong feeling about it, but lean towards bug fix. It didn't break any of our code (or at least any of our tests :-) ) so it seems safe from my perspective. Gary ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
Re: [Zope3-dev] Cleaning up the widget mess a little bit: bug or feature
Hi again, Gary Poster wrote: I don't have a very strong feeling about it, but lean towards bug fix. It didn't break any of our code (or at least any of our tests :-) ) so it seems safe from my perspective. I was trying to apply the patch to the 3.3 branch and noticed that the patch isn't compatible, as it requires a restructuring that happened on the trunk a while ago. This refactoring (r70331) introduces a very small feature, but the broken behaviour (trying to put anything into _toFormValue) exists in the old variant as well. I'd both apply r70331 and r71548 to 3.3 and 3.2 then. Christian -- gocept gmbh co. kg - forsterstraße 29 - 06112 halle/saale - germany www.gocept.com - [EMAIL PROTECTED] - phone +49 345 122 9889 7 - fax +49 345 122 9889 1 - zope and plone consulting and development signature.asc Description: OpenPGP digital signature ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
