Hi, Kristofer!
On May 11, Kristofer Pettersson wrote:
> Sergei Golubchik wrote:
>>>
>>> I'm removing the comment + the two null assignments for now.
>> please move them to construct_help_options and use strcmp("MyISAM" or
>> "HEAP") as before.
>> Then bug#44691 will only need to fix hard-coded "mandatory" list,
>> which is what this bug is about. And your bugfix won't break old
>> behavior too.
>
> I would agree on not breaking old behaviour, but what _is_ the old
> behaviour? I don't see any --myisam or --memory/--heap options if I do
> "mysqld --verbose --help", do you?
No, I don't. But I'm afraid that after your patch I will see them,
unless you will zero out comment field in construct_help_options().
> We can add a hard coded test as you suggest but it wouldn't change
> anything and I still need to change it in the patch for 44691, right?
It will preserve the old behavior (no --myisam and --memory) options.
And no, for 44691 this zero-out code won't need to be changed, only
hard-coded test will (I mean, only the test, but not the code executed
when the test succeeds).
Another comment to your patch: you've changed the option comment from
(innodb, for example)
--innodb Enable InnoDB plugin. Disable with --skip-innodb
(will save memory)
to
--innodb Enable or disable InnoDB plugin. Disable with --skip-innodb
(will save memory)
I think the old version is more clear. Indeed, --innodb will "enable"
the plugin, not "enable or disable" it.
Regards / Mit vielen Grüßen,
Sergei
--
__ ___ ___ ____ __
/ |/ /_ __/ __/ __ \/ / Sergei Golubchik <serg@stripped>
/ /|_/ / // /\ \/ /_/ / /__ Principal Software Engineer/Server Architect
/_/ /_/\_, /___/\___\_\___/ Sun Microsystems GmbH, HRB München 161028
<___/ Sonnenallee 1, 85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schroeder, Wolfgang Engels, Wolf Frenkel
Vorsitzender des Aufsichtsrates: Martin Häring
