Bug 47883

Summary: webkit-patch stats the filesystem too many times
Product: WebKit Reporter: Adam Barth <abarth>
Component: New BugsAssignee: Adam Barth <abarth>
Severity: Normal CC: commit-queue, eric
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Description Flags
Patch none

Description Adam Barth 2010-10-18 22:46:26 PDT
webkit-patch stats the filesystem too many times
Comment 1 Adam Barth 2010-10-18 22:48:52 PDT
Created attachment 71133 [details]
Comment 2 Eric Seidel (no email) 2010-10-19 08:02:47 PDT
Comment on attachment 71133 [details]

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

Seems OK.  Thanks.

> WebKitTools/Scripts/webkitpy/common/checkout/api.py:92
>          absolute_paths = [os.path.join(self._scm.checkout_root, path) for path in changed_files]

Seems we should just make a self._make_paths_absolute(paths) method, and then this entire _modified_files_matching_predicate function is redundant.

> WebKitTools/Scripts/webkitpy/common/checkout/api.py:96
> +        return self._modified_files_matching_predicate(git_commit, self._is_path_to_changelog, changed_files=changed_files)

This then becomes:
absolute_paths = self._make_paths_absolute(changed_files or self.scm.changed_files(git_commit))
return [path for path in absolute_paths if self._is_path_to_changelog(path)]

> WebKitTools/Scripts/webkitpy/tool/steps/abstractstep.py:55
>      _well_known_keys = {
> -        "diff": lambda self, state: self._tool.scm().create_patch(self._options.git_commit),
> -        "changelogs": lambda self, state: self._tool.checkout().modified_changelogs(self._options.git_commit),
>          "bug_title": lambda self, state: self._tool.bugs.fetch_bug(state["bug_id"]).title(),
> +        "changed_files": lambda self, state: self._tool.scm().changed_files(self._options.git_commit),
> +        "diff": lambda self, state: self._tool.scm().create_patch(self._options.git_commit, changed_files=self._changed_files(state)),
> +        "changelogs": lambda self, state: self._tool.checkout().modified_changelogs(self._options.git_commit, changed_files=self._changed_files(state)),

It seems State needs to be its own object.  These are just @property declarations for an object these days.

> WebKitTools/Scripts/webkitpy/tool/steps/editchangelog.py:38
> +        self.did_modify_checkout(state)

We could be smart here and check if they actually modified the ChangeLog. :)
Comment 3 WebKit Commit Bot 2010-10-19 08:21:22 PDT
Comment on attachment 71133 [details]

Clearing flags on attachment: 71133

Committed r70059: <http://trac.webkit.org/changeset/70059>
Comment 4 WebKit Commit Bot 2010-10-19 08:21:27 PDT
All reviewed patches have been landed.  Closing bug.