[dev] Branches (again), Horde 4.1/5, recent IMAP changes

Wed Nov 2 20:17:50 UTC 2011

Zitat von Michael M Slusarz <slusarz at horde.org>:

> Quoting Jan Schneider <jan at horde.org>:
>
>> Even though there are still some complaints, I think the switch to  
>> PEAR based releases was a great overall success. With a whopping  
>> 780 (seven hundred and eighty!) package releases since we started  
>> the new PEAR server, we brought the paradigm of "release early,  
>> release often" to new heights, at least for Horde's standards.  
>> That, and the implicit dependency management makes me never want to  
>> go back.
>
> Agreed.
>
>> But that's not the only thing we changed for Horde 4, we also  
>> intended to set new rules for release management  
>> (http://wiki.horde.org/Doc/Dev/ReleaseCycle) and branch management  
>> (http://wiki.horde.org/Doc/Dev/Branches). This is the area where we  
>> still need to improve.
>>
>> We are currently in our 2nd release cycle since we released Horde  
>> 4, and everybody noticed by now that there is no Horde 4.1 in the  
>> works, let alone Horde 5. The reason is that we simply didn't have  
>> any new features or larger changes in our stable code base that  
>> hadn't been released yet anyway. That's why we focused on releasing  
>> the "missing bits" during this release cycle, i.e. the until now  
>> unreleased applications that had to be ported to Horde 4.
>>
>> The reason for the lack of new features is that we kept adding new  
>> stuff to the master branch, i.e. to the stable branch, and still  
>> shuffled things around with new stable releases. I let this happen,  
>> despite this being against the release/branch rules, because I felt  
>> like there still were some teething troubles with our young Horde 4  
>> release (and Michael even explicitly expressed that he considered  
>> our stable release too early at one point) We also had some  
>> important things missing originally, that shouldn't had to wait  
>> another half a year. But at the same time I was intent to get an  
>> agreement to more strictly enforce these rules once the 2nd release  
>> cycle is over.
>> Because the flip-side of the coin is that (even though some of  
>> those changes in stable releases were necessary to fix bugs), the  
>> "stable" releases were much less stable than they should have been.  
>> Too often some of those larger changes caused intermediate  
>> regressions. Fortunately this didn't have such a high impact,  
>> thanks to our new release model (see above). I still think we  
>> should stop this now though. People should not be prepared to  
>> experience regressions due to code restructuring (like the mailbox  
>> encoding in IMP), notable UI changes (like in dynamic Kronolith) or  
>> new features suddenly popping up (like in - everywhere), while they  
>> update within a minor version. Such changes need more testing  
>> through RCs (yeah, I know, they won't be tested properly anyway,  
>> but that's a different story), and with piling them up for the next  
>> minor version, we actually *have* some minor version to release.
>
> I don't necessarily disagree with this.  Two things to add:
>
> 1) As the person in charge of the IMP related stuff, the major  
> changes are/were necessary because they were critical bugs.  Fixing  
> a critical bug that results in spinoff bugs for the new code is,  
> unfortunately, unavoidable.  But I disagree with the statement that  
> it somehow made the code "less" stable.  A critical bug is a  
> critical bug.  Both major issues that cropped up with IMP (issues  
> with suhosin; not being able to access certain mailboxes due to  
> UTF8->UTF7IMAP conversions; POP3 access being completely broken with  
> certain POP3 servers) needed to be immediately fixed.  As an end  
> user, the inability to access mailboxes is a dealbreaker.

Well, it *did* make things less stable, because fixing a bug that only  
affected a *few* users (and that could be worked around with by  
disabling a PHP extension) broke certain features for *all* users.  
Thanks to the new release model this wasn't that dramatic because we  
were able to quickly release follow-up bug fixes. I am also not saying  
that we should have waited for working around this (it wasn't even a  
bug in Horde) until the next minor version, though I'm a bit torn on  
this one.
What I mean is that users should expect that we don't fix a bug while  
breaking something else with it, as long as the keep updating within a  
minor version. Sure, this can always happen, but the risk can be  
minimized by keeping large code changes like this one for the next  
minor version.

> 2) From my perspective, a lot of the new features that have been  
> added is a direct result of the limited release schedule for the  
> .0.0 releases.  There was a bunch of stuff that needed to be added,  
> or did not become apparent until feedback/bug fixes were made, that  
> did not seem appropriate to wait 6 months - year to add.

Yes, I think we all agree on this. Which doesn't mean we shouldn't try  
to change this in the future.

> I have been holding back changes related to UTF-8 mail sending, a  
> drafts refactor, and a blocks CSS refactor specifically for the next  
> version.  These are local - I find the 'develop' branch very  
> difficult and unintuitive to use, so it is not helpful for me to  
> push there, and pushing topic branches is pretty pointless since  
> nobody really looks at them.

Topic branches are really only for developing new feature that take  
some time and that you *want* to or *need* to share with someone else.  
Well, at least pushed topic branches. You should still use topic  
branches locally, but that's your personal choice about how you want  
to develop. But this is better discussed in a reply to your CSS  
example from today.

>> Thus the recent Horde_Imap_Client_Mailbox changes make me very  
>> strong headaches. This is exactly the kind of changes that has  
>> potential of causing more trouble than solving problems (for a bug  
>> fix release, that is). It introduces new dependencies, touches  
>> large parts of important code in IMP, and even converts existing  
>> user preferences.
>
> I disagree.  I have been testing this code for about 3 weeks now -  
> this is not something I threw together at the last minute.  And it  
> fixes an extremely critical issue - it prevents ANY access to  
> certain mailboxes.  That is completely unacceptable.
>
> These changes do NOT change backward compatibility at all with the  
> Imap_Client code.  I specifically made sure this was the case, even  
> though it needlessly complicates the code.

It's been running a few days now on all developers' machines, so it  
indeed seems to be quite stable. Are you fine with releasing new  
versions of Imap_Client and IMP today or tomorrow? Or should we do RCs  
just to be sure?

> Not to bring this up again, but I will again raise my frustrations  
> with working on IMP, especially as it relates to critical code  
> changes.  *NO* other application relies so heavily on Horde-level  
> code - e.g. MIME, IMAP, Text_Filtering code - and that makes it  
> extremely difficult to fix certain items.  It takes me many times  
> longer to fix an issue, simply because I have to go through the  
> mental gymnastics of having to figure out how not to break existing  
> APIs.  IMP should not be less stable, or less able to be bugfixed,  
> simply because it has more of a reliance on global Horde libraries  
> but unfortunately, that is the case.

Acknowledged. :) But that's a fact we (or you) have to live with I'm afraid.

>> This is unacceptable for a simple bug fix release IMO. I *strongly*  
>> suggest to revert this on master, and re-apply it to the develop  
>> branch, so that it gets more and longer testing and won't be  
>> released before a IMP 5.1 RC.
>
> Please look at the commit again.  There is almost ZERO code changed  
> within IMP.  The changed code essentially consists of removing all  
> UTF8 -> UTF7-IMAP string conversions and converting all mailbox  
> parameters passed to Horde_Imap_Client to the object  
> representations.  And all of these changes are made in a single  
> method within a single library (IMP_Imap), which is why I designed  
> it this way.  Neither is likely to add bugs that are more critical  
> than the bug being solved.
>
> The only major difference in IMP is the preferences/config options  
> changing.  But I felt that it was a necessary evil for several  
> reasons:
>
> 1) The fixed_folders option is not heavily used (there was a fairly  
> major issue related to this I recently fixed which most likely would  
> have resulted in broken behavior, and I had not heard anything about  
> it).  To require the tiny percentage of people using this config  
> option - and not only using this config option, but also having at  
> least one of the mailboxes encoded in UTF7-IMAP - was not enough to  
> prevent a critical fix from happening.
>
> 2) The prefs options are a bit of a closer call, but my guess is  
> that most people are using the defaults and, even if not using the  
> defaults, the vast majority of local entries do not use UTF7-IMAP  
> mailboxes.  Both of these changes are clearly indicated in UPGRADING.
>
> However, we can't wait several more months to apply these changes.   
> The impetus for me fixing this critical bug was because I was bit by  
> it several weeks ago.   Created one of the mailboxes that cannot be  
> accessed given the current code - as an enduser, this would  
> potentially be a dealbreaker.
>
> Obviously, this decision can be disagreed with and I respect that.   
> But as I mentioned back during the first H4 release discussion, I am  
> against slavishly following release management guidelines where  
> critical bug fixes are necessary.  If 5.1 can be released within 1-2  
> weeks, I will gladly pull this out.  But that doesn't seem to be a  
> realistic timeframe.

Jan.

-- 
Do you need professional PHP or Horde consulting?
http://horde.org/consulting/