On 13 Mar 2007, at 14:28, Pete Harlan wrote:
> Thank you very much for reviewing my patch. I'll come up with another
> one to address your points, and I'll submit it with a feature-request
> bug report as you suggest.
Hi Pete. Great! To be clear for others, you did the right thing in
mailing first. I'd rather see the idea posted before a bug filed to
track it.
> I do have two questions:
>
>>> - "NO_ENGINE_SUBSTITUTION",
>>> + "NO_ENGINE_SUBSTITUTION", "EMPTY_DEFAULT_FOR_BLOB",
>>
>> This and the next are very minor, but if you can avoid removing a
>> line and adding a line, then we're sure to avoid merging conflicts.
>
> Sure.
>
>>> /*HIGH_NOT_PRECEDENCE*/ 19,
>>> - /*NO_ENGINE_SUBSTITUTION*/ 22
>>> + /*NO_ENGINE_SUBSTITUTION*/ 22,
>>> + /*EMPTY_DEFAULT_FOR_BLOB*/ 22
>>
>> (again)
>
> In this case, I need to add a "," between the previous entry and mine.
> I could instead write ",22", to avoid changing an existing line; does
> avoiding merge conflicts trump coding style in something like this, at
> least during patch review?
Keep in mind that it doesn't have to go at the end of the list. If
you add "22," before the end, then you avoid the problem altogether.
But in general, if order of elements is significant and you must add
them at the end, then legibility and style trumps merge-ease.
> Also, is a unified diff perfectly fine? Most diffs I've seen around
> MySQL are done with -c. I'm not in the habit of using -c, but if it
> makes it easier for the right folks to read patches I can change my
> unified ways.
Either unified or context are fine by me. I don't know many people
with a strong preference among those two, so I think it's safe to say
you may pick one.
- chad
--
Chad Miller, Software Developer chad@stripped
MySQL Inc., www.mysql.com
Orlando, Florida, USA 13-20z, UTC-0500
Office: +1 408 213 6740 sip:6740@stripped
Attachment: [application/pgp-signature] This is a digitally signed message part PGP.sig
