[webkit-reviews] review denied: [Bug 106402] Adding tool to check a branch from git was successfully landed. : [Attachment 183331] Patch

[bugzilla-daemon at webkit.org]

Dirk Pranke <dpranke at chromium.org> has denied Tim 'mithro' Ansell
<mithro at mithis.com>'s request for review:
Bug 106402: Adding tool to check a branch from git was successfully landed.
https://bugs.webkit.org/show_bug.cgi?id=106402

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

------- Additional Comments from Dirk Pranke <dpranke at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=183331&action=review


> Tools/ChangeLog:6
> +	   This command compares the finally commit patch to the changes which

nit: commit -> committed.

This is actually a bit more detail than I would've written :). I'd probably
just have put something like "was successfully landed by comparing the local
diff to what was actually committed".

Note that I'm only being so picky here because you're new to the project and
I'm trying to give some guidelines; I hope this is helpful.

> Tools/Scripts/webkitpy/tool/steps/haslanded.py:88
> +	       ["interdiff", landed_file.name, local_file.name],
decode_output=False)

looks like interdiff is only available on my ubuntu precise box by default (not
mac or win), so you might want to trap this in case the commands' not available
and log something useful.

This approach seems reasonable and I'm fine w/ you landing something along
these lines to see if it actually works and is useful. Needs at least some
minimal testing of the new command, and you need to figure out what's up with
the change to the diff_parser before you get an r+, though.