[webkit-reviews] review requested: [Bug 41333] VCSUtils.pm complains about uninitialized value $newLine : [Attachment 60832] Patch with unit test

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has asked  for review:
Bug 41333: VCSUtils.pm complains about uninitialized value $newLine
https://bugs.webkit.org/show_bug.cgi?id=41333

Attachment 60832: Patch with unit test
https://bugs.webkit.org/attachment.cgi?id=60832&action=review

------- Additional Comments from Daniel Bates <dbates at webkit.org>
We run off the end of an array when processing a change log entry that was
inserted earlier in the file, but after an entry with the same author and date
(because of the blank line of context at the top of the change log entry).

Currently, when we detect that the new change log entry is earlier in the
change log file we do not move this entry to the top of the file as the entry
may have been explicitly placed earlier by the author of the patch. This
situation seems rare and we may want to re-consider this decision and/or
provide some kind of optional boolean flag to forcefully move a new change log
entry to the top of the change log to prevent inadvertently landing a change
with a change log entry in the wrong place. Alternatively, we may want to
consider adding a warning message of the form "WARNING: The new change log
entry is not at the top of the change log. Make sure this is what you intended"
when we detect such a change log entry.

For the purpose of this patch, I have decided to only fix the array issue as I
think it is best that we resolve the placement issue/print a warning in a
separate patch.