Merge lp:~popey/ubuntu-terminal-app/add-snapcraft-config into lp:~ubuntu-terminal-dev/ubuntu-terminal-app/reboot
| Status: | Merged |
|---|---|
| Approved by: | David Planella on 2016-09-15 |
| Approved revision: | 209 |
| Merged at revision: | 208 |
| Proposed branch: | lp:~popey/ubuntu-terminal-app/add-snapcraft-config |
| Merge into: | lp:~ubuntu-terminal-dev/ubuntu-terminal-app/reboot |
| Diff against target: |
61 lines (+38/-1) 2 files modified
snapcraft.yaml (+37/-0) src/plugin/qmltermwidget/CMakeLists.txt (+1/-1) |
| To merge this branch: | bzr merge lp:~popey/ubuntu-terminal-app/add-snapcraft-config |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| David Planella | 2016-09-08 | Approve on 2016-09-15 | |
| Jenkins Bot | continuous-integration | Approve on 2016-09-15 | |
|
Review via email:
|
|||
Commit Message
Adds snapcraft.yaml to do builds of ubuntu-
Also fixes broken xenial build - thanks to Dan Chapman
Description of the Change
Adds snapcraft.yaml to do builds of ubuntu-
Also fixes broken xenial build - thanks to Dan Chapman
| David Planella (dpm) wrote : | # |
Nice work!
I've added some comments. Right now, if I understand it correctly, the terminal will only run on Unity 7, whereas we'd like to get it running on Unity 8.
I'm just adding it as a reminder, which shouldn't block this MP to get approved after reviewing the comments, but it's something to have a look at on the next iteration.
To get it running on Unity 8 will either require a wrapper [1] or the desktop-launcher to include these lines from the wrapper (plus a couple of extra dependencies, which IIRC did not add much to the size of the snap).
[1] https:/
| David Planella (dpm) wrote : | # |
I've added another comment, but I also just noticed that this branch attempts to fix the same thing, but adding the CMake rule somewhere else:
https:/
| Dan Chapman ξΏ (dpniel) wrote : | # |
Added a comment inline.
| David Planella (dpm) wrote : | # |
Replied inline again, thanks!
| Alan Pope πΊπ§π± π¦ (popey) wrote : | # |
Thanks for the comments - fixing!
- 209. By Alan Pope πΊπ§π± π¦ on 2016-09-15
-
Set click mode, tidy out comments, remove unnecessary delete from cmake, as per comments.
PASSED: Continuous integration, rev:209
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
| David Planella (dpm) wrote : | # |
Looks good to me, thanks!
I think we can merge it as it is. CLICK_MODE=on is not necessary and installs extra files [1], but it seems the snap works, so we might as well leave it as it is for a first pass.


FAILED: Continuous integration, rev:208 /code.launchpad .net/~popey/ ubuntu- terminal- app/add- snapcraft- config/ +merge/ 305206/ +edit-commit- message
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:/
https:/ /core-apps- jenkins. ubuntu. com/job/ terminal- app-ci/ 57/ /core-apps- jenkins. ubuntu. com/job/ generic- update- mp/972/ console
Executed test runs:
None: https:/
Click here to trigger a rebuild: /core-apps- jenkins. ubuntu. com/job/ terminal- app-ci/ 57/rebuild
https:/