[Webkit-unassigned] [Bug 130676] Fix commit-log-editor bug revealed by r165447

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Mar 25 03:38:58 PDT 2014


https://bugs.webkit.org/show_bug.cgi?id=130676


Csaba Osztrogonác <ossy at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #227661|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #2 from Csaba Osztrogonác <ossy at webkit.org>  2014-03-25 03:39:18 PST ---
(From update of attachment 227661)
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?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


More information about the webkit-unassigned mailing list