[webkit-reviews] review granted: [Bug 28291] webkit-patch (or a pre-commit hook) needs to prevent bad ChangeLog changes : [Attachment 77387] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Eric Seidel <eric at webkit.org>'s
request for review:
Bug 28291: webkit-patch (or a pre-commit hook) needs to prevent bad ChangeLog
changes
https://bugs.webkit.org/show_bug.cgi?id=28291

Attachment 77387: Patch
https://bugs.webkit.org/attachment.cgi?id=77387&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=77387&action=review

Did you want to add this step to any of the sequences?

> Tools/Scripts/webkitpy/tool/steps/checkchangelogs.py:37
> +# This is closely related to the ValidateReviewer step and the
CommitterValidator class
> +# we may eventually want to unify more of this code in one place.
> +class CheckChangeLogs(AbstractStep):

Maybe call it ValidateChangeLogs then?

> Tools/Scripts/webkitpy/tool/steps/checkchangelogs.py:41
> +	       Options.git_commit,

Isn't this option added to all steps by AbstractStep?

> Tools/Scripts/webkitpy/tool/steps/checkchangelogs.py:55
> +	   # FIXME: Do we need to make the file path absolute?
> +	   self._tool.scm().diff_for_file(diff_file.filename)

scm takes paths relative to the root of the repo.  Did you try this from a
non-repo root CWD?