arphaman added a comment. Some comments:
- I had to make a couple of post-commit fixes to appease the buildbots when I committed your first patch, so it looks like this one doesn't apply cleanly anymore. Can you please rebase it? - Let's separate the addition of HTML output and the improvements to the algorithm in two patches. I would recommend that you leave the algorithm improvements in this one and post a follow-up patch that has HTML output (with a HTML output test). - I won't be able to accept a change like this without tests. Did you monitor the changes in the HTML output while your worked on improvements to the algorithm? Please try and base the tests on those changes and improvements. ================ Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:814 + return 0.001 // + + 0.5 * Options.MinSimilarity * haveSameParents(M, Id1, Id2) // + + 0.5 * Options.MinSimilarity * SameValue // ---------------- What are these magic constants? Please describe them in comments and factor out to appropriately named const variables if needed. https://reviews.llvm.org/D34748 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
