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?
| 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.
| > 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.
| > 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.
We cannot continue to have this battle every time you create a new
patch.
And from glancing over the patch there are several things that could
have been broken out into separate patches.
| 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.
| 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.
--
Lgb
