Closed
Bug 1189858
Opened 10 years ago
Closed 10 years ago
[mozprofile] investigate why mozprofile requires manifestparser, and try to remove that
Categories
(Testing :: Mozbase, defect)
Testing
Mozbase
Tracking
(firefox42 affected)
RESOLVED
FIXED
| Tracking | Status | |
|---|---|---|
| firefox42 | --- | affected |
People
(Reporter: parkouss, Assigned: parkouss)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
|
3.78 KB,
patch
|
ahal
:
review+
|
Details | Diff | Splinter Review |
|
1.06 KB,
patch
|
ahal
:
review+
|
Details | Diff | Splinter Review |
mozprofile requires manifestparser, and that dependency doesn't make any sense intuitively.
I think we should have a look and try to fix this.
| Assignee | ||
Comment 1•10 years ago
|
||
The manifestparser dependency of mozprofile is made optionnal with this patch.
It is still possible to install it using the extra pip/setuptools syntax:
> pip install mozprofile[manifest]
Note that the following:
> pip install mozprofile
won't install nor requires manifestparser now.
I forgot to say this in the first comment, but it will be good for outside tools like mozregression to not require this as it is useless for them.
Note that I updated the mozprofile version in the same patch, so I could use that version from mozregression.
I don't think this would break internal tests, as everything in mozbase is zipped and accessible I think. Anyway, we'll see that on try.
Assignee: nobody → j.parkouss
Status: NEW → ASSIGNED
Attachment #8642281 -
Flags: review?(ahalberstadt)
Comment 2•10 years ago
|
||
Comment on attachment 8642281 [details] [diff] [review]
1189858.patch
Review of attachment 8642281 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks! Lgtm.
::: testing/mozbase/mozprofile/setup.py
@@ +30,5 @@
> license='MPL 2.0',
> packages=['mozprofile'],
> include_package_data=True,
> zip_safe=False,
> + install_requires=['mozfile >= 1.0', 'mozlog >= 3.0'],
Please keep the deps variable, it's easier to find at the top of the file and by now everyone is trained to look for it there :).
Attachment #8642281 -
Flags: review?(ahalberstadt) → review+
| Assignee | ||
Comment 3•10 years ago
|
||
Thanks Andrew! Fixed, and pushed to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f7abe1e52326
| Assignee | ||
Comment 5•10 years ago
|
||
Leave-open so I can release that to pypi once landed in m-c.
Keywords: leave-open
Comment 6•10 years ago
|
||
| Assignee | ||
Comment 7•10 years ago
|
||
Oh, I tested before doing the release - and I made a typo in the last patch...
Sorry for that - this new patch fixes it, I tested it locally with success (specifying or not with pip the extra dependency).
Attachment #8643654 -
Flags: review?(ahalberstadt)
Updated•10 years ago
|
Attachment #8643654 -
Flags: review?(ahalberstadt) → review+
Comment 9•10 years ago
|
||
| Assignee | ||
Comment 10•10 years ago
|
||
Uploaded on pypi:
Submitting dist/mozprofile-0.26.tar.gz to https://pypi.python.org/pypi
Server response (200): OK
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 11•8 years ago
|
||
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in
before you can comment on or make changes to this bug.
Description
•