Merge lp:~xnox/ubuntu-terminal-app/drop-lxml-dep into lp:~dpm/ubuntu-terminal-app/merge-plugin

Proposed by Dimitri John Ledkov
Status: Rejected
Rejected by: David Planella
Proposed branch: lp:~xnox/ubuntu-terminal-app/drop-lxml-dep
Merge into: lp:~dpm/ubuntu-terminal-app/merge-plugin
Diff against target: 38 lines (+1/-8)
3 files modified
README.md (+0/-6)
debian/control (+0/-1)
tests/autopilot/ubuntu_terminal_app/CMakePluginParser.py (+1/-1)
To merge this branch: bzr merge lp:~xnox/ubuntu-terminal-app/drop-lxml-dep
Reviewer Review Type Date Requested Status
David Planella Needs Fixing
Nicholas Skaggs (community) Approve
Review via email: mp+218482@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Please test. Note that autopilot package still shipps modules in python2 location, instead of python3. I'll make another merge proposal to fix that.

Revision history for this message
Nicholas Skaggs (nskaggs) :
review: Approve
Revision history for this message
David Planella (dpm) wrote :

Note that it's not a matter of just replacing the import, as it seems that the standard module is not a drop-in replacement for lxml.

So further code changes will be required in CMakePluginParser.py to make it work.

In any case, as discussed on IRC with balloons, I'll split the original MP to isolate this change and not block on it.

review: Needs Fixing
Revision history for this message
Dimitri John Ledkov (xnox) wrote :

On 6 May 2014 19:46, David Planella <email address hidden> wrote:
> Review: Needs Fixing
>
> Note that it's not a matter of just replacing the import, as it seems that the standard module is not a drop-in replacement for lxml.
>
> So further code changes will be required in CMakePluginParser.py to make it work.
>
> In any case, as discussed on IRC with balloons, I'll split the original MP to isolate this change and not block on it.

Yeah, well i've run the tests on the desktop and they all passed....
not sure if CMakePluginParser was actually in fact used or not.

--
Regards,

Dimitri.

Unmerged revisions

71. By Dimitri John Ledkov on 2014-05-06

Drop lxml dependency

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'README.md'
2--- README.md 2014-05-06 15:07:24 +0000
3+++ README.md 2014-05-06 17:44:27 +0000
4@@ -150,9 +150,3 @@
5
6 cd tests/autopilot
7 autopilot run ubuntu_terminal_app
8-
9-To run the tests on a device, the python3-lxml lib needs to be manually copied
10-to /home/phablet/autopilot. Note that the armhf version is required, as it's a
11-compiled module. This manual step is only temporary until the
12-phablet-click-test-setup gains the capability of automatically doing this
13-setup.
14
15=== modified file 'debian/control'
16--- debian/control 2014-05-06 15:07:24 +0000
17+++ debian/control 2014-05-06 17:44:27 +0000
18@@ -45,7 +45,6 @@
19 Depends: ${misc:Depends},
20 libautopilot-qt,
21 libqt5test5,
22- python3-lxml,
23 ubuntu-terminal-app (>= ${source:Version}),
24 ubuntu-ui-toolkit-autopilot,
25 Description: Autopilot tests for Terminal Application
26
27=== modified file 'tests/autopilot/ubuntu_terminal_app/CMakePluginParser.py'
28--- tests/autopilot/ubuntu_terminal_app/CMakePluginParser.py 2014-04-28 06:06:48 +0000
29+++ tests/autopilot/ubuntu_terminal_app/CMakePluginParser.py 2014-05-06 17:44:27 +0000
30@@ -23,7 +23,7 @@
31 """
32
33 import sys
34-from lxml import etree
35+import xml.etree.ElementTree as etree
36
37
38 class CMakePluginParseError(Exception):

Subscribers

People subscribed via source and target branches