Sergei, On Sat, Oct 14, 2017 at 9:20 PM, Sergei Golubchik <s...@mariadb.org> wrote: > Hi, Aleksey! > > On Oct 13, Aleksey Midenkov wrote: >> Hello! >> >> Do we have to write test cases for such coding errors? Because this is >> developer factor: either he does such errors or not. What are >> conditions of getting PR merged? > > Generally it's preferrable to have a test case for all bugs where it's > possible. > > In your particular PR, it's unlikely that someone will intentionally add > memcpy() back. But it's possible that lines around this memcpy would be > modified in one of the earlier branches, and during the merge git would > automerge by restoring memcpy(). Or that a person resolving the merge > conflict would restore the memcpy() without realizing the consequences. > > So, yes, test case is better than no test case. > > Sometimes a test case is extremely complex or just impossible to create. > So it's generally at the discretion of the reviewer. > > But in this case you're saying that this will be tested in the > versioning.partition test. Perhaps you can simply push it together with > versioning.partition test instead of pushing it separately?
Mmm... I'm afraid that it is reproducible only with versioning code. And I don't know if it will be reproducible when we remove dependence on native partitioning. I would suggest to move towards C++ generally, i.e. make constructors instead of memset(), memcpy(). Making it as coding guide will eventually rectify such cases. Anyway, I guess it's better with PR than without it... ;) > > Regards, > Sergei > Chief Architect MariaDB > and secur...@mariadb.org -- All the best, Aleksey Midenkov Tempesta Technologies _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp