Merge lp://qastaging/~jassmith/unity/unity.fix-alt-tab-progression into lp://qastaging/unity

Proposed by Jason Smith
Status: Merged
Merged at revision: 2025
Proposed branch: lp://qastaging/~jassmith/unity/unity.fix-alt-tab-progression
Merge into: lp://qastaging/unity
Diff against target: 12 lines (+1/-1)
1 file modified
plugins/unityshell/src/SwitcherController.cpp (+1/-1)
To merge this branch: bzr merge lp://qastaging/~jassmith/unity/unity.fix-alt-tab-progression
Reviewer Review Type Date Requested Status
Gord Allott (community) Needs Fixing
Thomas Voß (community) Needs Information
Review via email: mp+93923@code.qastaging.launchpad.net

Description of the change

Makes unity always move to the next app when reaching the end of a window list and pressing tab whilst in the switcher

To post a comment you must log in.
Revision history for this message
Thomas Voß (thomas-voss) wrote :

Looks good to me, but tests are missing to make sure that this actually fixes the bug.

review: Needs Information
Revision history for this message
Gord Allott (gordallott) wrote :

needs tests

review: Needs Fixing
Revision history for this message
Martin Mrazik (mrazik) wrote :

Hi,

Jason -- I branched your branch, added a manual testcase and did a merge proposal. Please merge the branches so we can move with this. I will try to automate this later. I actually believe this can be automated easily with autopilot.

On a side note -- this needs tests for two main reasons:

1. Every change in the code (no matter how small or big; no matter if we are adding or deleting code; etc) has the potential to break something. To minimize the risk we write tests.

2. We want to test for regressions. I can easily imagine that in a year or so this bug will be totally forgotten and at the same time somebody (who is not even canonical employee yet) will think this behavior is weird and rolls it back. At the same time some other reviewer (who has also no clue about this bug) will tell himself -- yeah, why not. And hurray, we have a regression and it will take yet another cycle or two to find it and fix it. If we automate this and add the big # to the test-case we can easily document that this is a bug and why its a bug.

3. (bonus reason) It is a policy

As a general note -- please don't hesitate to come to me asking for some help with the tests. What we are doing here is really an engineering best practice that really works. I can understand you are overwhelmed by other tasks and in such case I can try to help you (like in this specific case).

Revision history for this message
Jason Smith (jassmith) wrote :

Im not opposed to testing, originally I was having gord mark all my merges as needing tests, then I realized I could just do that myself :)

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.