[Webkit-unassigned] [Bug 41153] rebaseline-chromium-webkit-tests should add or remove files to local git repository

[bugzilla-daemon at webkit.org]

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


Shinichiro Hamaji <hamaji at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #59732|review?                     |review-
               Flag|                            |




--- Comment #6 from Shinichiro Hamaji <hamaji at chromium.org>  2010-06-24 22:51:42 PST ---
(From update of attachment 59732)
Looks great. Putting r- for some nicpicks.

WebKitTools/Scripts/webkitpy/common/checkout/scm.py: 
 +  
I think we need this blank line:

"Separate top-level function and class definitions with two blank lines."

http://www.python.org/dev/peps/pep-0008/


WebKitTools/Scripts/webkitpy/common/checkout/scm.py:349
 +      def _add_directories_recursively(self, path):
I would say _add_parent_directories or something. "recursively" sounds you will add a directory and its descendants.

WebKitTools/Scripts/webkitpy/common/checkout/scm.py:352
 +          parent, here = os.path.split(path)
Cannot we just use os.path.dirname here?

WebKitTools/Scripts/webkitpy/common/checkout/scm.py:349
 +      def _add_directories_recursively(self, path):
Also, I would write a docstring here.

WebKitTools/Scripts/webkitpy/common/checkout/scm.py:458
 +      def pset(self, pname, pvalue, path):
I would use propset as the name of this function.

WebKitTools/Scripts/webkitpy/common/checkout/scm.py:462
 +      def pget(self, pname, path):
propget

WebKitTools/ChangeLog:11
 +          - add "-U" and "-q" options to rebaseline_chromium_webkit_tests.py
This can be done in a separate change, but it's OK for now. Please consider splitting your patch next time.

WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py:477
 +                      svn_error = True
This variable should be renamed to scm_error?

WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py:478
 +                  if not svn_error and isinstance(self._scm, scm.SVN):
I will comment on this separately.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.