webbrowser-app

Merge lp:~canonical-platform-qa/webbrowser-app/fix1444170-flake8 into lp:webbrowser-app

fix1444170-flake8
Merge into trunk

Proposed by Leo Arias on 2015-04-16

Status:

Merged

Approved by:

Olivier Tilloy on 2015-04-24

Approved revision:

972

Merged at revision:

993

Proposed branch:

lp:~canonical-platform-qa/webbrowser-app/fix1444170-flake8

Merge into:

lp:webbrowser-app

Diff against target:

49 lines (+9/-8)

3 files modified

debian/control (+1/-1)
tests/autopilot/webapp_container/tests/test_app_launch.py (+7/-6)
tests/unittests/sanity/CMakeLists.txt (+1/-1)

To merge this branch:

bzr merge lp:~canonical-platform-qa/webbrowser-app/fix1444170-flake8

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Olivier Tilloy		2015-04-16	Approve on 2015-04-24
PS Jenkins bot	continuous-integration		Needs Fixing on 2015-04-22
Review via email: mp+256522@code.launchpad.net

Commit message

Fixed flake8 error.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-16:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1657/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-16:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1662/rebuild

review: Needs Fixing (continuous-integration)

lp:~canonical-platform-qa/webbrowser-app/fix1444170-flake8 updated on 2015-04-17

970. By Leo Arias on 2015-04-17: Updated the test for flake8 to use py3.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-17:

FAILED: Continuous integration, rev:970
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1667/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2314/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/424/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/424/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/424/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2312/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1667/rebuild

review: Needs Fixing (continuous-integration)

lp:~canonical-platform-qa/webbrowser-app/fix1444170-flake8 updated on 2015-04-17

971. By Leo Arias on 2015-04-17: Use python3-flake8.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-17:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1669/rebuild

review: Approve (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-04-18:

That looks good to me. It’s really weird that when run with python2 flake8 doesn’t complain…
One tiny minor nitpick:

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-04-20:

Why is webapp_install_path surrounded with parentheses?

lp:~canonical-platform-qa/webbrowser-app/fix1444170-flake8 updated on 2015-04-22

972. By Leo Arias on 2015-04-22: Use dash to separate copyright years.

Revision history for this message

Leo Arias (elopio) wrote on 2015-04-22:

> 21 +# Copyright 2014, 2015 Canonical
>
> in the rest of the codebase, we use dashes, so that would be "Copyright
> 2014-2015 Canonical"

According to some boring legalese from the fsf, it might be safer to use commas. Take a look at the last sentence of the sixth paragraph in https://www.gnu.org/licenses/gpl-howto.html

This is extremely unlikely to be a big deal, but still makes me prefer commas. And I prefer consistency over everything else, so for now lets go with dashes.

Revision history for this message

Leo Arias (elopio) wrote on 2015-04-22:

> Why is webapp_install_path surrounded with parentheses?

That's to wrap the line. Without parentheses, the line would be longer than allowed by pep8. We could wrap with a \, but:

"The preferred way of wrapping long lines is by using Python's implied line continuation inside parentheses, brackets and braces. Long lines can be broken over multiple lines by wrapping expressions in parentheses. These should be used in preference to using a backslash for line continuation. "

https://www.python.org/dev/peps/pep-0008/

Revision history for this message

Leo Arias (elopio) wrote on 2015-04-22:

and I forgot to say thanks for the review :)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-22:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1678/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-04-24:

Thanks.
Regarding the boring legalese, I understand why you originally used a comma, but I’m also +1 on consistency across the codebase (this is something we can change globally at a later point), and besides the app has actively been developed since its inception, so copyright applies to the 2013-2015 period.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Canonical Platform QA Team

Ubuntu Phablet Team

to status/vote changes:

Rachel Liu

1	=== modified file 'debian/control'
2	--- debian/control 2015-04-10 13:33:10 +0000
3	+++ debian/control 2015-04-22 18:56:06 +0000
4	@@ -8,8 +8,8 @@
5	hardening-wrapper,
6	liboxideqt-qmlplugin (>= 1.4),
7	libqt5sql5-sqlite,
8	- python-flake8,
9	python3-all,
10	+ python3-flake8,
11	qml-module-qtquick2 (>= 5.4) \| qtdeclarative5-qtquick2-plugin (>= 5.4),
12	qml-module-qttest \| qtdeclarative5-test-plugin,
13	qt5-default,
14
15	=== modified file 'tests/autopilot/webapp_container/tests/test_app_launch.py'
16	--- tests/autopilot/webapp_container/tests/test_app_launch.py 2015-01-06 19:03:18 +0000
17	+++ tests/autopilot/webapp_container/tests/test_app_launch.py 2015-04-22 18:56:06 +0000
18	@@ -1,5 +1,5 @@
19	# -- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 --
20	-# Copyright 2014 Canonical
21	+# Copyright 2014-2015 Canonical
22	#
23	# This program is free software: you can redistribute it and/or modify it
24	# under the terms of the GNU General Public License version 3, as published
25	@@ -88,11 +88,12 @@
26	args = ["--webapp='dGVzdA=='"]
27	rule = 'MAP *.test.com:80 ' + self.get_base_url_hostname()
28	with generate_temp_webapp() as webapp_install_path:
29	- self.launch_webcontainer_app(
30	- args,
31	- {'UBUNTU_WEBVIEW_HOST_MAPPING_RULES': rule,
32	- 'WEBAPP_QML_DEFAULT_WEBAPPS_INSTALL_FOLDER':
33	- webapp_install_path})
34	+ env_vars = {
35	+ 'UBUNTU_WEBVIEW_HOST_MAPPING_RULES': rule,
36	+ 'WEBAPP_QML_DEFAULT_WEBAPPS_INSTALL_FOLDER': (
37	+ webapp_install_path)
38	+ }
39	+ self.launch_webcontainer_app(args, env_vars)
40	webview = self.get_oxide_webview()
41	webapp_url = 'http://www.test.com/'
42	self.assertThat(webview.url, Eventually(Equals(webapp_url)))
43
44	=== modified file 'tests/unittests/sanity/CMakeLists.txt'
45	--- tests/unittests/sanity/CMakeLists.txt 2014-11-12 17:00:41 +0000
46	+++ tests/unittests/sanity/CMakeLists.txt 2015-04-22 18:56:06 +0000
47	@@ -1,1 +1,1 @@
48	-add_test(flake8 flake8 ${autopilot-tests_SOURCE_DIR})
49	+add_test(flake8 python3 -m flake8 ${autopilot-tests_SOURCE_DIR})

webbrowser-app

Merge lp:~canonical-platform-qa/webbrowser-app/fix1444170-flake8 into lp:webbrowser-app

Commit message

Description of the change

Preview Diff

Subscribers