Closed
Bug 827600
Opened 12 years ago
Closed 11 years ago
mozcrash has functions that should live in mozfile
Categories
(Testing :: Mozbase, defect)
Testing
Mozbase
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: k0scist, Unassigned)
References
Details
Attachments
(1 file)
5.50 KB,
patch
|
wlach
:
review+
|
Details | Diff | Splinter Review |
mozcrash has a function, extractAll, that we already have in mozfile: https://github.com/mozilla/mozbase/blob/master/mozfile/mozfile/mozfile.py#L48 https://github.com/mozilla/mozbase/blob/master/mozcrash/mozcrash/mozcrash.py#L21 mozcrash should depend on mozfile and use the version in there. In addition, mozcrash has a function is_url: https://github.com/mozilla/mozbase/blob/master/mozcrash/mozcrash/mozcrash.py#L10 This should probably be put in mozfile since we use this all over the place in various forms (usually i just do `'://' in thing`). Both packages should be version bumped and re-released to pypi
Reporter | ||
Comment 1•11 years ago
|
||
following the versions of mozfile and mozcrash should be bumped
Attachment #700140 -
Flags: review?(wlachance)
Comment 2•11 years ago
|
||
Comment on attachment 700140 [details] [diff] [review] fix LGTM except: >diff --git a/mozcrash/mozcrash/mozcrash.py b/mozcrash/mozcrash/mozcrash.py >index f22e06e..35fca1c 100644 >+def is_url(thing): >+ """ >+ Return True if thing looks like a URL. >+ """ >+ # We want to download URLs like http://... but not Windows paths like c:\... This comment doesn't really make sense for a generic function. Could you put it in the relevant spot in mozcrash?
Attachment #700140 -
Flags: review?(wlachance) → review+
Reporter | ||
Comment 3•11 years ago
|
||
(In reply to William Lachance (:wlach) from comment #2) > Comment on attachment 700140 [details] [diff] [review] > fix > > LGTM except: > > >diff --git a/mozcrash/mozcrash/mozcrash.py b/mozcrash/mozcrash/mozcrash.py > >index f22e06e..35fca1c 100644 > >+def is_url(thing): > >+ """ > >+ Return True if thing looks like a URL. > >+ """ > >+ # We want to download URLs like http://... but not Windows paths like c:\... > > This comment doesn't really make sense for a generic function. Could you put > it in the relevant spot in mozcrash? Sure, sgtm
Reporter | ||
Comment 4•11 years ago
|
||
pushed: https://github.com/mozilla/mozbase/commit/47ea9337f311d517163082b37ce756d784ab3e2c
Reporter | ||
Updated•11 years ago
|
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•