Merge lp:~darran-kelinske/music-app/lp_bug_1428454 into lp:music-app/remix
| Status: | Merged | ||||
|---|---|---|---|---|---|
| Approved by: | Andrew Hayzen on 2015-03-24 | ||||
| Approved revision: | 853 | ||||
| Merged at revision: | 849 | ||||
| Proposed branch: | lp:~darran-kelinske/music-app/lp_bug_1428454 | ||||
| Merge into: | lp:music-app/remix | ||||
| Diff against target: |
146 lines (+75/-1) 4 files modified
MusicPlaylists.qml (+1/-0) common/SongsPage.qml (+2/-0) tests/autopilot/music_app/__init__.py (+24/-0) tests/autopilot/music_app/tests/test_music.py (+48/-1) |
||||
| To merge this branch: | bzr merge lp:~darran-kelinske/music-app/lp_bug_1428454 | ||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Ubuntu Phone Apps Jenkins Bot | continuous-integration | Approve on 2015-03-24 | |
| Andrew Hayzen | 2015-03-19 | Approve on 2015-03-24 | |
|
Review via email:
|
|||
Commit Message
* Added a test case for deleting a playlist
Description of the Change
Added a test case for deleting a playlist
| dazza5000 (darran-kelinske) wrote : | # |
Thank yuo for the quick feedback! I think I got the changes. Please let me know. :)
- 852. By Darran Kelinske <email address hidden> on 2015-03-20
-
fixing more spacing issues
| Andrew Hayzen (ahayzen) wrote : | # |
This is looking closer :) Unfortunately there are still some failures...
Some of them are just because there is whitespace at the end of the line.
Also there is one inline comment.
$ pep8 tests/
tests/autopilot
tests/autopilot
tests/autopilot
tests/autopilot
tests/autopilot
$ pyflakes tests/
tests/autopilot
def test_artists_
I have made a diff [0] which contains fixes for the issues stated above. Then once it is passing pep8/pyflakes we can try triggering jenkins :)
- 853. By Darran Kelinske <email address hidden> on 2015-03-21
-
additional formatting/syntax fixes :)
FAILED: Continuous integration, rev:853
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https:/
http://
Executed test runs:
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Andrew Hayzen (ahayzen) wrote : | # |
Awesome, thanks for submitting this :) Looks good to me!


Looks good so far :) Just a few inline comments...
L65, L75, L188 and possibly L140 - Could you remove this extra blank lines? (See PEP8 styling on this [0] - 2 lines for a class def and 1 for a method)
L102 - Is there any reason for renaming this var and method call? Could we keep it as add_to_playlist singular?
As I don't think jenkins runs for non-team members, I've manually run PEP8/pyflakes for you so you can see the errors [1]
0 - https:/ /www.python. org/dev/ peps/pep- 0008/#blank- lines pastebin. ubuntu. com/10629706/
1 - http://