[webkit-reviews] review granted: [Bug 47883] webkit-patch stats the filesystem too many times : [Attachment 71133] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Oct 19 08:02:46 PDT 2010
Eric Seidel <eric at webkit.org> has granted Adam Barth <abarth at webkit.org>'s
request for review:
Bug 47883: webkit-patch stats the filesystem too many times
https://bugs.webkit.org/show_bug.cgi?id=47883
Attachment 71133: Patch
https://bugs.webkit.org/attachment.cgi?id=71133&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
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. :)
More information about the webkit-reviews
mailing list