[Webkit-unassigned] [Bug 138373] Add the ability to search for modifications that are staged for commit in git.py

[bugzilla-daemon at webkit.org]

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

--- Comment #7 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 241000
  --> https://bugs.webkit.org/attachment.cgi?id=241000
Updated patch for getting modified files staged for commit.

View in context: https://bugs.webkit.org/attachment.cgi?id=241000&action=review

I know that this patch was already reviewed and landed. I had some questions about implementation, unit tests, and usage of this code.

Currently modifications_staged_for_commit() is unused. I take it you plan to make use of this functionality in a follow up patch. Usually it's preferred to add new code together with a call site both to help the reviewer better understand the goal of the patch as well and to ensure that the added code doesn't inadvertently remain unused (say, if future plans change).

> Tools/Scripts/webkitpy/common/checkout/scm/git.py:210
> +    def modifications_staged_for_commit(self):

Maybe a better name for this function would be modified_files_staged_for_commit? This would make the name of this function more closely match the name of other functions (e.g. changed_files).

> Tools/Scripts/webkitpy/common/checkout/scm/git.py:215
> +        # This will only return non-deleted files with the "updated in index" status
> +        # as defined by http://git-scm.com/docs/git-status.
> +        status_command = [self.executable_name, 'status', '--short']
> +        updated_in_index_regexp = '^M[ M] (?P<filename>.+)$'
> +        return self.run_status_and_extract_filenames(status_command, updated_in_index_regexp)

I didn't have a chance to read <http://git-scm.com/docs/git-status> yet. I take it that it doesn't make sense to write this function as:

def modified_files_staged_for_commit(self):
    status_command = [self.executable_name, "diff", "--name-status", "--no-renames", "--cached"]
    return self.run_status_and_extract_filenames(status_command, self._status_regexp("M"))

?

If it does not make sense to implement this function as mentioned above then I suggest we add a unit test for this function. See file <http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py>.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141105/90e69a7a/attachment-0002.html>