maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxxx>]

Hi, Oleksandr!

On Mar 29, Oleksandr Byelkin wrote:
>>
>>> +#define PLUGIN_MATURITY_TEST 1
>>> +#define PLUGIN_MATURITY_ALPHA 2
>>> +#define PLUGIN_MATURITY_BETA 3
>>> +#define PLUGIN_MATURITY_GAMMA 4
>>> +#define PLUGIN_MATURITY_RELEASE 5
>>
>> "release" ? or production or stable ?
>> there can be an alpha release, or beta release.
>> obviously, "release" does not indicate a degree of maturity.
>>
>> and I don't like "test" either.
>> PLUGIN_MATURITY_TEST is just not readable. I'd rather drop it and start
>> with alpha.

> I agree with changing RELEASE with STABLE, but test looks logical for me 
> (and for Monty also as far he invented it).

"maturity test" ? You test somebody's maturity ? :)
I think the name is badly chosen and confusing.
I my opinion, "alhpa" is enough. or just let them use UNKNOWN for
something which is worse than alpha.

>>> @@ -94,15 +112,29 @@
>>>
>>>
>>> #ifndef MYSQL_DYNAMIC_PLUGIN
>>> +
>>> #define __MYSQL_DECLARE_PLUGIN(NAME, VERSION, PSIZE, DECLS) \
>>> int VERSION= MYSQL_PLUGIN_INTERFACE_VERSION;                \
>>> int PSIZE= sizeof(struct st_mysql_plugin);                  \
>>> struct st_mysql_plugin DECLS[]= {
>>> +
>>> +#define __MARIA_DECLARE_PLUGIN(NAME, VERSION, PSIZE, DECLS) \
>>
>> don't use names that start from __, the C99 standard says
>> "All identifiers that begin with an underscore and either an uppercase 
>> letter
>> or another underscore are always reserved for any use."
>> (ISO/IEC 9899:1999, 7.1.3 Reserved Identifiers)
>>
>> It's unfortunate that __MYSQL_DECLARE_PLUGIN breaks this rule,
>> but it's not a reason for us to do the same
>
> So what will be better, one or three or remove '_' at all?

three ___ still start from __ :)
I meant, just remove leading __.

If you want to highligh that the macro is for internal use, you can, for
example, add __ at the end, or add _impl or something:

  MARIA_DECLARE_PLUGIN__
  MARIA_DECLARE_PLUGIN_impl

>>> === modified file 'sql/ha_ndbcluster.cc'
>>> --- a/sql/ha_ndbcluster.cc	2009-09-07 20:50:10 +0000
>>> +++ b/sql/ha_ndbcluster.cc	2010-03-11 15:02:03 +0000
>>> @@ -10561,5 +10561,23 @@
>>>   NULL                        /* config options                  */
>>> }
>>> mysql_declare_plugin_end;
>>> +maria_declare_plugin(ndbcluster)
>>> +{
>>> +  MYSQL_STORAGE_ENGINE_PLUGIN,
>>> +  &ndbcluster_storage_engine,
>>> +  ndbcluster_hton_name,
>>> +  "MySQL AB",
>>> +  "Clustered, fault-tolerant tables",
>>> +  PLUGIN_LICENSE_GPL,
>>> +  ndbcluster_init, /* Plugin Init */
>>> +  NULL, /* Plugin Deinit */
>>> +  0x0100 /* 1.0 */,
>>> +  ndb_status_variables_export,/* status variables                */
>>> +  NULL,                       /* system variables                */
>>> +  "1.0",                      /* string version */
>>> +  PLUGIN_MATURITY_BETA,       /* maturity */
>>
>> beta ? says who ?
>
> My guess reviewed by Monty.

let's call it gamma (discussed on IRC)

>>> +  NULL                        /* config options                  */
>>> +}
>>> +maria_declare_plugin_end;
>>> === modified file 'sql/sql_plugin.cc'
>>> --- a/sql/sql_plugin.cc	2009-11-12 04:31:28 +0000
>>> +++ b/sql/sql_plugin.cc	2010-03-11 15:02:03 +0000
.....
> I saw your bugfix but left it for merge, usually it is not good to fix the 
> same things in different paches

I just mean that during the merge you need to remember to fix them in
your copy-pasted function, bzr won't do it automatically.

>>> === modified file 'storage/pbxt/src/ha_pbxt.cc'
>>> --- a/storage/pbxt/src/ha_pbxt.cc	2009-09-03 06:15:03 +0000
>>> +++ b/storage/pbxt/src/ha_pbxt.cc	2010-03-11 15:02:03 +0000
...
>>> +{ /* PBXT_STATISTICS */
>>> +  MYSQL_INFORMATION_SCHEMA_PLUGIN,
>>> +  &pbxt_statitics,
>>> +  "PBXT_STATISTICS",
>>> +  "Paul McCullagh, PrimeBase Technologies GmbH",
>>> +  "PBXT internal system statitics",
>>> +  PLUGIN_LICENSE_GPL,
>>> +  pbxt_init_statitics,	      /* plugin init */
>>> +  pbxt_exit_statitics,	      /* plugin deinit */
>>> +  0x0005,
>>> +  NULL,			      /* status variables */
>>> +  NULL,			      /* system variables */
>>> +  "1.0.09g RC3",              /* string version */
>>> +  PLUGIN_MATURITY_GAMMA,      /* maturity */
>>> +  NULL			      /* config options */
>>> +}
>>> +maria_declare_plugin_end;
>>> +#endif
>>
>> Paul needs to know about this to update his repository
>
> I think letter is enough, right?

Yes.

Regards,
Sergei