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

List:       kwrite-devel
Subject:    Re: [kate/frameworks] part: port away from QAbstractItemModel::reset()
From:       Dominik Haumann <dhaumann () kde ! org>
Date:       2013-12-19 22:00:52
Message-ID: 3028499.HXWLXUtmoh () eriador
[Download RAW message or body]

On Thursday 19 December 2013 22:29:22 Michal Humpula wrote:
> On Thursday 19 of December 2013 22:01:44 Milian Wolff wrote:
> > On Thursday 19 December 2013 20:57:49 Dominik Haumann wrote:
> > > Hi Milian,
> > > 
> > > On Thursday 19 December 2013 20:36:44 Milian Wolff wrote:
> > > > On Thursday 19 December 2013 17:58:25 Michal Humpula wrote:
> > > > > Git commit e6374518f5a966b8badda3a5e90fb873cd7ec08e by Michal
> > > > > Humpula.
> > > > > Committed on 19/12/2013 at 17:54.
> > > > > Pushed by michalhumpula into branch 'frameworks'.
> > > > > 
> > > > > port away from QAbstractItemModel::reset()
> > > > 
> > > > This is potentially dangerous as it can easily lead to undesired
> > > > side-effects, esp. performance wise.
> > > > 
> > > > See also: https://git.reviewboard.kde.org/r/113917/
> > > 
> > > So can you be more constructive about what exactly to do here?
> > > Saying it's "potentially dangerous" does not help much ;)
> > > 
> > > Will this review resolve this, and when we merge master into frameworks
> > > we
> > > don't need Michals changes anymore?
> > 
> > I mean that instead of adding reset blindly to KF5, we should get the
> > above
> > mentioned review request into a usable state first.
> > 
> > It's not merged for a reason (namely performance impact).
> > 
> > Bye
> 
> Ok, I'm close to done with porting. So I will look at that review. After
> fixing the master, we can cherry-pick for KF5, ok?

That sounds like a good idea to me :-)

Thanks!
Dominik
_______________________________________________
KWrite-Devel mailing list
KWrite-Devel@kde.org
https://mail.kde.org/mailman/listinfo/kwrite-devel
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic