[Webkit-unassigned] [Bug 50901] Add remote zip file handling to webkitpy.

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from James Kozianski <koz at chromium.org>  2011-01-03 19:01:35 PST ---
(In reply to comment #2)
> (From update of attachment 76349 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=76349&action=review
> 
> So I feel like you're trying to make a directory act like a zip, when you really want a zip to act like a directory.  I'm confused as to the overarching design here.
> 
> You want some abstraction to treat both local results as well as remote results the same, right?  As well as zipped results vs. non-zipped, right?
> 
> Why not just always download and unzip and then they're all teh same?  I'm confused.
> 
> > WebKitTools/ChangeLog:7
> > +
> 
> The ChangeLog should talk more about the "why" of these changes.  Why are these two classes needed?

Done.

> 
> > WebKitTools/ChangeLog:9
> > +        * Scripts/webkitpy/common/diraszip.py: Added.
> > +        * Scripts/webkitpy/common/net/remotezip.py: Added.
> 
> Every change should have a test.  In python it's super easy.  Just name a file with _unittest.py in the webkitpy directory and test-webkitpy will find it and run all the tests in it.

Done.

> 
> > WebKitTools/Scripts/webkitpy/common/diraszip.py:36
> > +    """Provides a zipfile-like interface to a local directory"""
> 
> So I'm confused why this is needed?
> 
> > WebKitTools/Scripts/webkitpy/common/diraszip.py:39
> > +        self._filesystem = filesystem if filesystem != None else FileSystem()
> 
> Using "None" to mean "default value" is often written as:
> if not filesystem:
>     filesystem = FileSystem()
> 
> and then later:
> self._filesystem = filesystem
> 
> But your solution works too.  I wouldn't have bothered checking explicit != None.  I probably would have "cheated" with or and just written: self._filesystem = filesystem or FileSystem()

Actually, I like your way the best, so I've changed it to that :-)

> 
> > WebKitTools/Scripts/webkitpy/common/diraszip.py:41
> > +        if not self._path.endswith(os.path.sep):
> > +            self._path += os.path.sep
> 
> Unclear why this is necessary?
> 
> > WebKitTools/Scripts/webkitpy/common/diraszip.py:58
> > +        return ospath.relpath(os.path.join(dir, filename), dir) is not None
> 
> bool(EXPRESSION) or "not not EXPRESSION" are common ways of converting from a value to a bool (which are slightly less verbose than "is not None"

Done.

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