Bug 36891 - webkitpy: Refactor two os.path.relpath() replacements to use the same method
Summary: webkitpy: Refactor two os.path.relpath() replacements to use the same method
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Chris Jerdonek
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-31 10:18 PDT by Chris Jerdonek
Modified: 2010-03-31 14:46 PDT (History)
4 users (show)

See Also:


Attachments
Proposed patch (14.00 KB, patch)
2010-03-31 10:33 PDT, Chris Jerdonek
eric: review+
cjerdonek: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Jerdonek 2010-03-31 10:18:59 PDT
We reimplemented os.path.relpath() twice.  We should share code there.
Comment 1 Chris Jerdonek 2010-03-31 10:33:18 PDT
Created attachment 52185 [details]
Proposed patch
Comment 2 Chris Jerdonek 2010-03-31 10:42:49 PDT
FYI, I added a section to this page about consequences of not using Python 2.6:

http://trac.webkit.org/wiki/PythonGuidelines
Comment 3 Eric Seidel (no email) 2010-03-31 11:04:55 PDT
Comment on attachment 52185 [details]
Proposed patch

YAY!  But what the heck is "opsys".  Seems like a strange module name.
Comment 4 Eric Seidel (no email) 2010-03-31 11:05:45 PDT
Seems like we should update that page to say that our scripts require 2.5, period.
Comment 5 Chris Jerdonek 2010-03-31 11:09:26 PDT
(In reply to comment #3)
> (From update of attachment 52185 [details])
> YAY!  But what the heck is "opsys".  Seems like a strange module name.

Operating system.  My preference was "os", but that presented problems for other modules in the package calling "import os".  I'm open to other suggestions.
Comment 6 Chris Jerdonek 2010-03-31 11:10:49 PDT
(In reply to comment #4)
> Seems like we should update that page to say that our scripts require 2.5,
> period.

Thanks -- yeah, I just noticed that, too.  I'll change it.
Comment 7 Eric Seidel (no email) 2010-03-31 11:13:44 PDT
Why not just call the module relpath?
Comment 8 Chris Jerdonek 2010-03-31 11:21:47 PDT
(In reply to comment #7)
> Why not just call the module relpath?

I thought about that, but then figured there was going to be at least one more os-related method at some point.  I suppose we can always rename the file.  I'll change it to relpath.
Comment 9 Chris Jerdonek 2010-03-31 11:22:09 PDT
Comment on attachment 52185 [details]
Proposed patch

cq- so I can rename the file to relpath.
Comment 10 Eric Seidel (no email) 2010-03-31 11:29:22 PDT
Maybe a name to designate that we're expanding on missing pieces of OS, like osadditions or something like that if that would be more clear than osys/relpath?

In the end it doesn't matter.  Feel free to name it as you choose. :)
Comment 11 Chris Jerdonek 2010-03-31 14:46:12 PDT
Committed: http://trac.webkit.org/changeset/56870

I split the difference between os and relpath and chose ospath.