Lars Gullik Bjønnes wrote:
Abdelrazak Younes <[EMAIL PROTECTED]> writes:
| Lars Gullik Bjønnes wrote:
| > Abdelrazak Younes <[EMAIL PROTECTED]> writes:
| > | * frontends/Timeout:
| > | - emit() renamed to emitSignal() to avoid compiler confusion with Qt
| > | emit() function (change propagated to all frontend).
| > This I do not like.
|
| I agree on the principle but, come on, we don't have to be religious
| about such stupid things.
We should just accept the stupid things then?
Temporarily, yes, in my opinion.
| The timeout change is much easier and
| quicker that the change you are proposing.
"quicker" does not matter much to me when wanting to clean code.
Evolution is the key word. Eventually, we will reach the "clean code"
status. And my patch helps toward that goal.
| > Qt deserves a lot of flak for pushing such common names into
| > the global namespace.
| > The qt side of things should fix this, not other code.
| > #ifdef emit
| > #define qt_emit emit
| > #undef emit
| > #endif
| > And then use qt_emit in all qt code.
|
| No, there is a boost macro compatibility flag that I plan to use in
| the future for that. This flag will need a change of the "emit",
| "signals" and "slot" macros to "Q_EMIT", "Q_SIGNALS" and "Q_SLOT" or
| something like that. In the mean time, please accept my timeout::emit
| change that is really not a big deal.
Well... I think it is a big deal.
I think there are better thing in life than to fight about such stupid
things. I don't have the power to change your mind though ;-)
| > As for the patch... you are changing too much at the same time, at
| > least I feel overwhelmed, and unable to review the patch properly.
|
| I am sorry but this is what happens when you do API changes. Patch
| reviewing is not a sane method in this case because there are a lot of
| code moving. You have to apply the patch and _read_ the code...
|
No. this is not acceptable. You will have to restrain yourself to not
change lots of small things at the same time.
When I get some free time, I do as much as I can. By going the road you
think is better, the change I propose will need one month to be in
instead of one day.
We cannot continue to have this battle every time you create a new
patch.
Right, trusting me a tiny more bit would help.
And from glancing over the patch there are several things that could
have been broken out into separate patches.
Then I am stopping here because I am already investing too much time in
LyX. Feel free to split up the patch as you will so that you can review
it easily. But quite frankly I am sure that you *can* review the patch
if you really want to.
| Beside, the svn log is not clear enough?
Not the point.
| > It must be possible to split this patch into more manageable pieces.
|
| Maybe but I really don't see the advantage and it will require me to
| invest much more time working on this for nothing.
I surely see the advantage. Easier to review, easier to understand,
takes shorter time to review -> can actually result in faster
progress.
I strongly disagree. I have seen many patches in the list that were much
bigger than this one and didn't provoke the reaction my patches always
provoke from you. I know you are the principal architect of the LyX
kernel but IMHO, you really should loose some control over the project.
| Please try to read
| the code instead. If you prefer a branch, we could do that also.
Same problem with that. Too many changes in the same patch or branch
is equally bad.
Then, that's the end.
Abdel.
PS: Lars, this is email so I probably should precise that there is no
hard feeling at all in what I am saying. But I have to make a decision
so I am officially retired now.
