Merge lp://qastaging/~ted/unity-mir/ubuntu-app-launch into lp://qastaging/unity-mir

Proposed by Ted Gould
Status: Merged
Approved by: Gerry Boland
Approved revision: 226
Merged at revision: 227
Proposed branch: lp://qastaging/~ted/unity-mir/ubuntu-app-launch
Merge into: lp://qastaging/unity-mir
Diff against target: 202 lines (+35/-35)
4 files modified
debian/control (+2/-2)
src/modules/Unity/Application/CMakeLists.txt (+1/-1)
src/modules/Unity/Application/upstart/applicationcontroller.cpp (+30/-30)
tests/auto/modules/Unity/Application/CMakeLists.txt (+2/-2)
To merge this branch: bzr merge lp://qastaging/~ted/unity-mir/ubuntu-app-launch
Reviewer Review Type Date Requested Status
Gerry Boland (community) Approve
Sebastien Bacher (community) the debian changes Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email: mp+220905@code.qastaging.launchpad.net

Commit message

UAL name chance got Ubuntu App Launch

Description of the change

Didn't change the interface from the module, just the change needed with the library change.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Gerry Boland (gerboland) wrote :

Please add Checklist, including listing dependant branches:
https://wiki.ubuntu.com/Process/Merges/Checklists/Unity-Mir

Commit message needs expanding. Name change of what exactly?

Note we use "upstart" in many more places in the code, including a specific "upstart" directory in src/modules/Unity/Application/, an upstart namespace and comments & tests referring to "upstart" - so please update those too.

Higher level question: why is this launch tool "ubuntu" specific?

review: Needs Fixing
226. By Ted Gould

Wrong type name

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Sebastien Bacher (seb128) wrote :

the packaging changes look fine to me, thanks

review: Approve (the debian changes)
Revision history for this message
Ted Gould (ted) wrote :

On Mon, 2014-05-26 at 09:52 +0000, Gerry Boland wrote:

> Please add Checklist, including listing dependant branches:
> https://wiki.ubuntu.com/Process/Merges/Checklists/Unity-Mir

 * Are there any related MPs required for this MP to build/function as
expected? Please list.

https://code.launchpad.net/~ted/upstart-app-launch/rename/+merge/217819

 * Did you perform an exploratory manual test run of your code change and any related functionality?

Ensured that it builds and runs, expect to do more testing on the
landing PPA. Need to get all the packages working together to test.

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?

Asked for a packaging review

> Commit message needs expanding. Name change of what exactly?

Updated.

> Note we use "upstart" in many more places in the code, including a specific "upstart" directory in src/modules/Unity/Application/, an upstart namespace and comments & tests referring to "upstart" - so please update those too.

Well, those comments are still correct, we *are* using Upstart in those
places. I looked at them and they still seemed correct. I can change
them to "init system" but "Ubuntu" is the wrong substitution.

Revision history for this message
Gerry Boland (gerboland) wrote :

LGTM, approved

 * Did you perform an exploratory manual test run of the code change and any related functionality?
N - waiting for the silo to build everything ok
 * Did CI run pass? If not, please explain why.
N - package name change

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