Re: [PATCH]bug 44516 improved label and business card document creation

On Mon, Feb 27, 2012 at 1:10 AM, Winfried Donkers
<W.Donkers@dci-electronics.nl> wrote:
> Norbert Thiebaud wrote (25 februari 2012 11:26)
>
> > #define RC_LABFMT_BEGIN  (RC_ENVELP_BEGIN + 50)
> > -#define RC_LABFMT_END    (RC_ENVELP_BEGIN + 59)
> > +#define RC_LABFMT_END    (RC_ENVELP_BEGIN + 62)
> > ...
> > why the shuffling of constants here ?
>
> These constants give the ranges for UI-constants (controls, text
> labels, etc.). I have added two text labels and two input fields to
> the label dimensions tab in the label wizard dialog. These fout did
> not fit in the range provided, so I had to enlarge the range, with the
> consequence that all ranges after LABFMT had to be moved as well.

ok

[...]
> With hindsight it would probably have been better to make a constant for
> this colour. It is not good practice to leave such colour definitions in
> the code.

yes, my point exactly :-)
It is not too late to do the 'Right Thing'(tm) including in master.

> I hope my explanations will help you. I not, please say so.

yes, but the mix of various changes leading to such a big patch and
especially the removal of entry peppered in the middle of a massive
re-formating makes that, even with some diff + sed magic, I cannot
reduce that to something readable, so I cannot review it.

In order for _me_ to sign-off for 3.5 branch, I would need you to
break your patch along the lines discussed earlier...
But hey, maybe someone else will review and sign-off as is...

Norbert