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? Regards, Sergei Chief Architect MariaDB and security@mariadb.org