[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: KDE/kdelibs/kdecore/sonnet
From:       David Faure <faure () kde ! org>
Date:       2009-11-30 23:38:18
Message-ID: 200912010038.19054.faure () kde ! org
[Download RAW message or body]

On Tuesday 01 December 2009, Zack Rusin wrote:
> On Monday 30 November 2009 17:53:12 David Faure wrote:
> > On Monday 30 November 2009, Zack Rusin wrote:
> > > SVN commit 1056377 by zack:
> > >
> > > sonnet: rewrite text segmentation algorithm
> > >
> > > since its creation sonnet was broken with indic, asian, arabic and
> > > a lot of other languages that didn't use english-like alphabets.
> > > this commit removes the custom text segmentation algorithm and replaces
> > > it with a proper unicode tr29-11 algorithm found in qtextboundaryfinder
> > > and hopefully makes sonnet work with all languages in the world. let me
> > >  know if causes any regressions.
> >
> > Well, that's what unittests are for ;-)
> 
> Yea, it's why I added some when committing this unfortunately it's a bit
>  hard to cover peculiarities of every language out there =)
> 
> > It seems your changes introduce a regression, because
> >  sonnet/tests/test_core uses 100% CPU for a very very long time and never
> >  terminates.
> 
> Hmm, it works here. That test always took a bit of time. I just committed a
> trivial fixlet that should improve it though, does it make it better for
>  you?

Yep, seems to work now.

BUT: this code was committed at the worst possible time (*very* close to
beta1 tagging) and without review, breaking the freeze.
And obviously (from your own words) it's impossible to be sure the code 
doesn't introduce any regressions. Can you revert it for now and commit it 
again a month from now when trunk is open for kde-4.5?
I don't want to think about what will happen if we ship 4.4 with infinite loops 
in some languages :/

-- 
David Faure, faure@kde.org, http://www.davidfaure.fr
Sponsored by Nokia to work on KDE, incl. Konqueror (http://www.konqueror.org).
[prev in list] [next in list] [prev in thread] [next in thread]