On Apr 20, 2012, at 12:05 PM, Kentaro Hara wrote: > +1 to the idea that we should try to add a test for each change. That > being said, as rniwa mentioned, it is sometimes difficult to make a > test because > > (A) we do not come up with a test case > (B) we know that the code is unreachable (e.g. > https://bugs.webkit.org/show_bug.cgi?id=84377) > > What is the consensus for such cases? As long as the change improves > codebase and the benefit is explicitly described in ChangeLog, is it > OK to commit the patch? Or shouldn't we try to fix a "potential" bug > observed in static analysis?
FWIW the change cited above and its current ChangeLog seem ok to me. In some cases, it's useful and important for a constructor to leave a data member uninitialized (e.g. for performance reasons), but those cases are exceptions and would need to be documented. The ChangeLog also explains how the issue fixed here is unreachable in practice and also untestable. If this issue was spotted by a static analyzer, it would be good to mention that in the ChangeLog too. Regards, Maciej _______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
