[KParts] patch: don't emit urlChanged, when url is the same

[Michal Humpula]

On Tuesday 24 of December 2013 14:05:36 David Faure wrote:
> On Tuesday 24 December 2013 13:50:29 Michal Humpula wrote:
> > On Tuesday 24 of December 2013 13:22:05 David Faure wrote:
> > > On Tuesday 24 December 2013 12:30:56 Michal Humpula wrote:
> > > > Hi there,
> > > > 
> > > > as suggested without fully functional reviewboard, here is the little
> > > > patch
> > > > for discussion.
> > > > 
> > > > Emit urlChanged only when actually changing url trough setUrl. The
> > > > drawback
> > > > of this is that now the fast assignment is replaced by possibly
> > > > expensive
> > > > string comparison.
> > > 
> > > That's ok, this method is typically not called 10000 times per second.
> > > 
> > > > On the other hand, when the urls are the same, the emit
> > > > wont fire and whole bunch of potentially expensive operations
> > > > connected
> > > > to
> > > > that signal will not happen.
> > > 
> > > Yep. The patch makes a lot of sense, please commit it - at least to
> > > frameworks. Do you also need it in kdelibs master?
> > 
> > Nope, no need. Just to be sure, pushing to master branch of
> > 
> > git at git.kde.org:kparts
> 
> Yes.
> 
> Please fix the coding style first: the '{' belongs on the same line as the
> if().
> 
> The code in frameworks/* should be very consistent now, since I reformatted
> it all with astyle. So no excuse anymore for inconsistent coding style in
> new code :)
> 
> > But, if I have a green light, then I would like to change the second
> > callsite of urlChanged() as well. Please, see attached patch. The
> > reasoning
> > is the same as with setUrl.
> 
> Coding style: please use { ... } even for one-line statements.
> 
> Looks OK otherwise.

It's there. Thanks!

Cheers
Michal