Sergei Golubchik <[email protected]> writes: > Just one question and couple of suggestions below. > If you agree with them - please change accordingly and push!
Thanks for review! I believe I addressed all your suggestions. Comments below. - Kristian. >> + $base_dir= dirname($file) >> + unless defined($base_dir); > > why do you pass base_dir as an argument, instead of always using > basedir($file) ? It was a mistake. I was trying to preserve old behaviour, but misunderstood what that behaviour was. Changed to use basedir($file) always. >> + for my $sourced_file ($base_dir . '/' . $include, >> + $::glob_mysql_test_dir . '/' . $include) > > Can you also look in the suite dir ? In the order of > basedir(file), suite_dir, glob_mysql_test_dir. Yes, done. >> + my ($sub_tags, $sub_master_opts, $sub_slave_opts)= >> + get_tags_from_file($sourced_file, $base_dir); >> + push @$tags, @$sub_tags; >> + # Tests (rpl.rpl_ddl at least) rely on options being in reverse >> order >> + # of include :-/ Hence the unshift(). >> + unshift @$master_opts, @$sub_master_opts; >> + unshift @$slave_opts, @$sub_slave_opts; > > I think having options in the order of includes is more logical. > Better to sort includes in the rpl_ddl.test, and use push here. > It is more intuitive and less error-prone. Agree. It turns out what is really needed is that options from includes are added before options from main file, so main file can override options from includes (this is what rpl_ddl needs, include/have_innodb adds --innodb, then rpl_ddl.test adds --skip-innodb for the slave only). So I changed it so that options come in order of includes, but first from included files, last from including file. More logical indeed. - Kristian. _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
