libkwidgets (Re: patches on kcolors)

[Stephen Kelly]

Giorgos Tsiapaliwkas wrote:

> On 11 April 2012 14:55, Stephen Kelly <steveire at gmail.com> wrote:
>>
>> It's not clear which patch you're talking about. As far as I can see, you
>> didn't really post the patches individually, but just posted the moevs
>> that you made along with the CMakeLists changes you made (as one patch).
>>
> 
> yes it is one patch because I don't see why I should split this patch into
> more.
> I did them all in one patch because they depend on each other.
> 
> 
>> I'd suggest you try to move fewer things at once and make sure the entire
>> branch builds (not just the parts you move). You want to move the things
>> at the bottom of the dependency tree first.
>>
> 
> I can't  due to the buildsystem. Every move that I make creates a lot of
> undef refs.

Even on the frameworks branch? I probably fixed many of those on the 
frameworks branch already.

> 
> 
> 
>> Yes, you can post the patches here for review. The best would be to use
>> git show -M -C <sha1> to create the patches so we can review them and I
>> can do a
>> build to make sure it builds.
> 
> 
> What's the difference from what I am doing?
> I copy&paste the moves and I give you the diff which has the changes.
> 
> 
>> Probably not. Lets review it first.
> 
> 
> ok, I am waiting :)
> 
> 
>> >
>> > Have you seen my issue about splitting?
>> > Also with all this cherry pick now we can't merge my branch. Right?
>>
>> It's better that I cherry pick from your branch so that history remains
>> buildable, or that you move files, post patches for review, I'll do a
>> test build, and one of us push the result.
>>
> 
> I agree, I think that its better if you make the cherry pick.
> 
> 
>> I'm still not sure I understand the goals of your moves though. What
>> classes
>> do you still want to move? Do you have some kind of plan, or are you just
>> trying to move everything from kdeui a few classes at a time? Why not
>> just rename kdeui? Why rename kdeui? Do you plan on addressing any of the
>> causes of interdependencies?
>>
> 
> my work here at the frameworks started at the first KF day, you told me to
> do it like this
> and I did.

Oh, I don't remember that. I don't think I was even there for the first 
frameworks volunteer day. Maybe it was someone else.

> 
> 
>> I'd suggest that a better contribution would be to analyse what
>> dependencies
>> exist and why. For example, look at KLed. It depends on the colors stuff
>> that is now in kdeguiaddons. Does it need to?
> 
> 
> I don't consider me as the best person for this work.  Why?
> Because I don't trust my decisions, our frameworks is a heavy job and I
> can't take such calls.
> Also in order to this I have to study all that code, which requires a lot
> of time and I don't volunteer for
> that. I prefer someone who has the appropriate experience to tell me what
> to do.

I was hoping you'd at least try. Here's the answer as far as I can tell:

kled.cpp uses KColorUtils::overlayColors.

Lxr tells us that it is the only user, apart from some playground code.

http://lxr.kde.org/ident?i=overlayColors

So, there is a reasonable argument for copying the implementation of that 
method into kled.cpp. That would make the widget 'KDE-free' and we could 
have a tier1 widgets library. 

There are other widgets which similarly could have their dependencies on 
things removed or reduced, which would justify such a library. We should 
also put the kplotwidget in there too - it shouldn't be in a framework of 
its own. I'll do that stuff, but I'm saying that you can quite easily 
determine the dependencies and why they exist if you just open the file and 
look for the used code in lxr.

I think we need analysis like that more than we need to move classes around 
at the moment.

Thanks,

Steve.