Merge lp://staging/~saviq/unity8/fix-flake8 into lp://staging/unity8

Proposed by Michał Sawicz
Status: Merged
Approved by: Michał Sawicz
Approved revision: 1733
Merged at revision: 1741
Proposed branch: lp://staging/~saviq/unity8/fix-flake8
Merge into: lp://staging/unity8
Diff against target: 512 lines (+74/-53)
14 files modified
tests/autopilot/unity8/application_lifecycle/tests/test_application_lifecycle.py (+0/-2)
tests/autopilot/unity8/fixture_setup.py (+2/-0)
tests/autopilot/unity8/greeter/tests/__init__.py (+3/-1)
tests/autopilot/unity8/greeter/tests/test_args.py (+4/-2)
tests/autopilot/unity8/indicators/__init__.py (+8/-7)
tests/autopilot/unity8/indicators/tests/__init__.py (+1/-0)
tests/autopilot/unity8/indicators/tests/test_action_latency.py (+21/-18)
tests/autopilot/unity8/shell/emulators/dash.py (+15/-10)
tests/autopilot/unity8/shell/emulators/greeter.py (+0/-1)
tests/autopilot/unity8/shell/emulators/tutorial.py (+0/-1)
tests/autopilot/unity8/shell/tests/__init__.py (+1/-0)
tests/autopilot/unity8/shell/tests/test_lock_screen.py (+4/-4)
tests/autopilot/unity8/shell/tests/test_notifications.py (+14/-6)
tests/autopilot/unity8/shell/tests/test_tutorial.py (+1/-1)
To merge this branch: bzr merge lp://staging/~saviq/unity8/fix-flake8
Reviewer Review Type Date Requested Status
Leo Arias (community) Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Unity Team Pending
Review via email: mp+256510@code.staging.launchpad.net

Commit message

Fix flake8 warnings

Description of the change

 * Are there any related MPs required for this MP to build/function as expected? Please list.
N
 * Did you perform an exploratory manual test run of your code change and any related functionality?
Y
 * Did you make sure that your branch does not contain spurious tags?
Y
 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A
 * If you changed the UI, has there been a design review?
N/A

To post a comment you must log in.
Revision history for this message
Leo Arias (elopio) wrote :

Awesome, thanks saviq.

A couple of things:

46+ main_window = \
47+ unity_proxy.select_single(main_window_emulator.QQuickView)

This is correct, but people in python avoid the \ as much as possible. This is what they have recommended to me instead:

main_window = (
    unity_proxy.select_single(main_window_emulator.QQuickView))

I think it's uglier, but I do it for consistency. I won't complaint if you prefer the \ here.

322+ self.select_single(objectName="processingIndicator")\
323+ .visible.wait_for(False)

This should be split after the parenthesis, like:

self.select_single(
    objectName="processingIndicator").visible.wait_for(
        False)

or,

processing_indicator = self.select_single(objectName="processingIndicator")
processing_indicator.visible.wait_for(False)

I prefer using two statements.

495-from unity8.shell.emulators import tutorial # NOQA

This is needed, it populates the autopilot object registry, used to match QML elements to custom proxy objects. It has a comment in the line before mentioning that. The # NOQA should take care of the flake8 error.

Needs fixing, because that last removed import will make the tests fail.

review: Needs Fixing
Revision history for this message
Leo Arias (elopio) wrote :

Oh, also:

elopio@tangamandapio76:~/workspace/canonical/unity/unity8/reviews/saviq/fix-flake8$ python3 -m flake8 .
./.bazaar/plugins/makecheck_unity_phablet.py:30:48: E901 SyntaxError: invalid syntax
./tests/autopilot/unity8/indicators/__init__.py:72:17: E126 continuation line over-indented for hanging indent

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
1732. By Michał Sawicz

Fix more

1733. By Michał Sawicz

One more tweak

Revision history for this message
Leo Arias (elopio) wrote :

Looks good. Waiting for jenkins to confirm that the tutorial test is green again.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Leo Arias (elopio) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches