After https://trac.webkit.org/changeset/165447 commit-log-editor generates wrong commit log for rollout changes when more then one changelog is affected, for example: https://trac.webkit.org/changeset/165497 (landed manually) https://trac.webkit.org/changeset/165581 (landed manually) https://trac.webkit.org/changeset/165829 (landed by CQ) https://trac.webkit.org/changeset/165933 (landed by CQ)
Created attachment 227661 [details] Patch
Comment on attachment 227661 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227661&action=review It seems the fix works after testing on some changes, but r- for now because of inaccurate and misleading comments. > Tools/ChangeLog:11 > + (removeLongestCommonPrefixEndingInDoubleNewline): Change the longest prefix ending to \n, > + because after r165447 the block ending depends on who landed the patch. r165447 didn't change the block ending. Does the block ending depends on who landed the patch? Checking the referred commit logs, it isn't true. Otherwise the fix seems good on the referred commit logs, but this can't be the proper comment why it works. removeLongestCommonPrefixEndingInDoubleNewline --> It should be renamed too, because it doesn't check double newline anymore. > Tools/Scripts/commit-log-editor:307 > - push @result, normalizeLineEndings("\n", $endl) if !$first; > - $first = 0; > + push @result, normalizeLineEndings("\n", $endl); Could you explain why we don't need this extra new line for non-first blocks anymore?
Created attachment 229279 [details] Patch
Comment on attachment 229279 [details] Patch The comments are better and clear now, r=me.
Comment on attachment 229279 [details] Patch Clearing flags on attachment: 229279 Committed r167243: <http://trac.webkit.org/changeset/167243>
All reviewed patches have been landed. Closing bug.
This patch broke webkitpy tests. Please run webkitpy tests when making changes, and please add tests for your fixes. I'll land updated results.
Update the result in <http://trac.webkit.org/r167296>. I'm not sure if the new result is better, please take a look.
(In reply to comment #7) > This patch broke webkitpy tests. Please run webkitpy tests when making changes, and please add tests for your fixes. > > I'll land updated results. Sorry, I haven't thought if a perl script can break a webkitpy unit test. :) I filed a new bug report to fix this minor bug and to add more unit tests: https://bugs.webkit.org/show_bug.cgi?id=131727