Merge lp:~canonical-platform-qa/qtcreator-plugin-ubuntu/update_simpleui_qttest-2 into lp:qtcreator-plugin-ubuntu
| Status: | Superseded |
|---|---|
| Proposed branch: | lp:~canonical-platform-qa/qtcreator-plugin-ubuntu/update_simpleui_qttest-2 |
| Merge into: | lp:qtcreator-plugin-ubuntu |
| Prerequisite: | lp:~canonical-platform-qa/qtcreator-plugin-ubuntu/qmake-template-makeover-fix_install |
| Diff against target: |
248 lines (+58/-16) 17 files modified
share/qtcreator/templates/wizards/ubuntu/backend-app-cmake/app/tests/autopilot/displayName/main/__init__.py (+1/-1) share/qtcreator/templates/wizards/ubuntu/backend-app-qmake/appName/appName.desktop (+1/-1) share/qtcreator/templates/wizards/ubuntu/backend-app-qmake/wizard.xml (+1/-1) share/qtcreator/templates/wizards/ubuntu/goproject/src/displayName/main.go (+1/-1) share/qtcreator/templates/wizards/ubuntu/goproject/wizard.xml (+1/-1) share/qtcreator/templates/wizards/ubuntu/qtquick-app-qmake/appName/appName.qrc (+1/-1) share/qtcreator/templates/wizards/ubuntu/qtquick-app-qmake/appName/main.cpp (+1/-1) share/qtcreator/templates/wizards/ubuntu/qtquick-app-qmake/wizard.xml (+1/-1) share/qtcreator/templates/wizards/ubuntu/share/displayName.qmlproject (+1/-1) share/qtcreator/templates/wizards/ubuntu/simple-app-qmake/appName/appName.desktop (+1/-1) share/qtcreator/templates/wizards/ubuntu/simple-app-qmake/wizard.xml (+1/-1) share/qtcreator/templates/wizards/ubuntu/simple-app-qmlproject/Makefile (+1/-1) share/qtcreator/templates/wizards/ubuntu/simple-app-qmlproject/displayName.desktop (+1/-1) share/qtcreator/templates/wizards/ubuntu/simple-app-qmlproject/tests/autopilot/displayName/__init__.py (+1/-1) share/qtcreator/templates/wizards/ubuntu/simple-app-qmlproject/tests/qml/tst_main.qml (+41/-0) share/qtcreator/templates/wizards/ubuntu/simple-app-qmlproject/wizard.xml (+2/-1) share/qtcreator/templates/wizards/ubuntu/simple-i18n-cmake/app/tests/autopilot/displayName/main/__init__.py (+1/-1) |
| To merge this branch: | bzr merge lp:~canonical-platform-qa/qtcreator-plugin-ubuntu/update_simpleui_qttest-2 |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| PS Jenkins bot | continuous-integration | Approve on 2015-02-04 | |
| Benjamin Zeller (community) | 2015-01-17 | Approve on 2015-01-21 | |
| Thomi Richards (community) | Approve on 2015-01-20 | ||
| Christopher Lee (community) | Needs Fixing on 2015-01-20 | ||
| Allan LeSage | 2015-01-17 | Pending | |
| Richard Huddie | 2015-01-17 | Pending | |
| Ubuntu SDK team | 2015-01-17 | Pending | |
|
Review via email:
|
|||
This proposal has been superseded by a proposal from 2015-02-04.
Commit Message
Added a qttest to simpel-
Description of the Change
Renamed the directory tests/unit to tests/qml, because generally we will have qml tests that are not unit tests.
Made the test actually pass. For that, I had to rename the main.qml to Main.qml because "main" is parsed as a property, not as a component. As the instructions to run the qml are shared by all the templates, I had to rename every main.qml to Main.qml.
Use UbuntuTestCase.
| Christopher Lee (veebers) wrote : | # |
Looking good. Just a couple of minor nit-picks though.
| Leo Arias (elopio) wrote : | # |
Thanks for the review veebers. I replied and will push a new version now.
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:354
http://
Executed test runs:
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Christopher Lee (veebers) wrote : | # |
Hey elopio, it looks like your editor or something has introduced a bunch of whitespace changes to the changelog. Can you revert these please as it adds a bunch of noise to the MP (unless this isn't an issue to the owning team).
| Leo Arias (elopio) wrote : | # |
veebers, I intentionally have my emacs configured to remove the extra white spaces. I do this to follow the boyscout rule [1] of leaving everything you touch a little cleaner than before, and because with our painful release process, it's not likely that branches to only fix small details like this are going to be released.
The downside is that the diff gets larger. When it's already too big, I try not to do it. But I think this one is not too big. I found that the main place where the spaces were removed was on the changelog, which I checked in by mistake. I reverted that one.
[1] http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:355
http://
Executed test runs:
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Thomi Richards (thomir) wrote : | # |
> veebers, I intentionally have my emacs configured to remove the extra white
> spaces. I do this to follow the boyscout rule [1] of leaving everything you
> touch a little cleaner than before, and because with our painful release
> process, it's not likely that branches to only fix small details like this are
> going to be released.
Leo, the boy scout rule doesn't say you need to land all those changes together.
I agree with veebers - please land separate fixes in separate branches. The bzr 'pipelines' plugin can help you if you want to make all these changes in one working directory.
Why? It introduces noise to the diff. The more noise there is, the harder it is to determine what needs to be scrutinised in a review. This MP already has several changes that are hard to follow ('main.qml' -> 'Main.qml' for example, which is easy to miss).
> The downside is that the diff gets larger. When it's already too big, I try
> not to do it. But I think this one is not too big. I found that the main place
> where the spaces were removed was on the changelog, which I checked in by
> mistake. I reverted that one.
It's not about diff size, it's about reviewer fatigue.
Cheers,
| Thomi Richards (thomir) wrote : | # |
A few things need fixing, plus please remove whitespace-only changes.
| Leo Arias (elopio) wrote : | # |
> It's not about diff size, it's about reviewer fatigue.
>
> Cheers,
I don't agree, but two against one.
Pushed with the white spaces reverted.
| Leo Arias (elopio) wrote : | # |
> A few things need fixing, plus please remove whitespace-only changes.
Thanks for the comments. I've pushed a new rev, please rereview.
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:358
http://
Executed test runs:
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:359
http://
Executed test runs:
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://

PASSED: Continuous integration, rev:353 jenkins. qa.ubuntu. com/job/ qtcreator- plugin- ubuntu- ci/497/ jenkins. qa.ubuntu. com/job/ qtcreator- plugin- ubuntu- vivid-amd64- ci/31 jenkins. qa.ubuntu. com/job/ qtcreator- plugin- ubuntu- vivid-amd64- ci/31/artifact/ work/output/ *zip*/output. zip jenkins. qa.ubuntu. com/job/ qtcreator- plugin- ubuntu- vivid-armhf- ci/31
http://
Executed test runs:
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/qtcreator- plugin- ubuntu- ci/497/ rebuild
http://