Currently mozversion dependencies are: dependencies = ['mozdevice >= 0.44', 'mozfile >= 1.0', 'mozlog >= 3.0'] It does not make sense to require mozdevice here. And I am fighting pypi nightmare dependencies because of that when installing talos locally (and talos do not require mozdevice at all now!).
:davehunt, do you mind if I fix this ? How is mozversion used around ?
mozversion uses mozdevice to determine the version information on Firefox OS devices: http://hg.mozilla.org/mozilla-central/file/32712cd01159/testing/mozbase/mozversion/mozversion/mozversion.py#l185
Well, I am not asking to remove any code, just make the dependency optional (sorry, that was not clear enough). We can use pip extra dependencies to still be able to require it explicitly from a pip install command, something like: > pip install mozversion[remote] See https://pythonhosted.org/setuptools/setuptools.html#declaring-extras-optional-features-with-their-own-dependencies.
I see, that makes sense. I wonder if the feature should be 'device' rather than 'remote' though? We should also update the documentation, and notify as many consumers of mozversion for Firefox OS that we're aware of. I've CC'd a few that come to mind.
Created attachment 8642353 [details] [diff] [review] 1189847.patch Thanks Dave! I followed your recommendation, using "device" as a feature name. I also added some documentation, and the generation is OK locally. I am not sure if you should be reviewer here, feel free to redirect the r? if it is not the case. :)
Assignee: nobody → j.parkouss
Status: NEW → ASSIGNED
Attachment #8642353 - Flags: review?(dave.hunt)
Comment on attachment 8642353 [details] [diff] [review] 1189847.patch Review of attachment 8642353 [details] [diff] [review]: ----------------------------------------------------------------- With the nit addressed this looks good to me. ::: testing/mozbase/docs/mozversion.rst @@ +5,5 @@ > provides version information such as the application name and the changesets > that it has been built from. This is commonly used in reporting or for > conditional logic based on the application under test. > > +Note that mozversion can report the version of remote devices (e.g. FirefoxOS) Nit: Firefox OS
Attachment #8642353 - Flags: review?(dave.hunt) → review+
Created attachment 8642403 [details] [diff] [review] 1189847.patch Thanks! Fixed. :) I will push that to try later. I'm not sure what try syntax I should use, I was thinking about one desktop and one b2g device build with one mochitest and one reftest test (in case they use that). If you have a better idea, please tell me.
try is green.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox42: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.