Merge lp:~attente/indicator-keyboard/fcitx-transition into lp:indicator-keyboard
| Status: | Merged |
|---|---|
| Approved by: | Sebastien Bacher on 2015-02-20 |
| Approved revision: | 434 |
| Merged at revision: | 670 |
| Proposed branch: | lp:~attente/indicator-keyboard/fcitx-transition |
| Merge into: | lp:indicator-keyboard |
| Diff against target: |
1817 lines (+450/-796) 17 files modified
.bzrignore (+11/-8) configure.ac (+1/-32) data/Makefile.am (+4/-4) debian/control (+2/-0) deps/Fcitx-1.0.metadata (+7/-0) deps/GnomeDesktop-3.0.metadata (+1/-0) deps/accountsservice.vapi (+0/-151) deps/fontconfig.vapi (+0/-13) deps/freetype2.vapi (+0/-20) deps/gnome-desktop-3.0.vapi (+0/-320) deps/pangoft2.vapi (+0/-33) lib/Makefile.am (+5/-2) lib/ibus-menu.vala (+13/-17) lib/indicator-menu.vala (+50/-43) lib/main.vala (+249/-94) lib/source.vala (+106/-59) tests/indicator-keyboard-test.in (+1/-0) |
| To merge this branch: | bzr merge lp:~attente/indicator-keyboard/fcitx-transition |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Allison Lortie (community) | Approve on 2015-03-03 | ||
| Sebastien Bacher | Approve on 2015-02-20 | ||
| PS Jenkins bot | continuous-integration | Approve on 2015-02-19 | |
| Ted Gould (community) | 2014-08-06 | Approve on 2014-09-16 | |
| csslayer | 2015-02-13 | Pending | |
|
Review via email:
|
|||
Commit Message
Basic support for Fcitx input sources.
Description of the Change
Basic support for Fcitx input sources.
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:411
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:412
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:413
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:420
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| Ted Gould (ted) wrote : | # |
So I read through this and I don't see any issues, but I don't really know how fcitx works, so any of the interactions are not known by me. Not sure if there's someone better to review this or not.
| Ted Gould (ted) wrote : | # |
I'll approve, but not top-approve in hoping there's someone else who can review better.
| Ted Gould (ted) wrote : | # |
Talking with Will this is going to have to be a 15.04 feature. I'm marking the review as a WIP as it'll need to be resubmitted to the 15.04 branch anyway.
| William Hua (attente) wrote : | # |
Hi Ted, could we please reconsider this for approval now that the Fcitx MIR is complete? I'll see if I can find another reviewer for it, maybe from the Kylin team.
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:422
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| Allison Lortie (desrt) wrote : | # |
Without really appreciating what's going on with the input method side of this patch, see below about some code comments.
- 423. By William Hua on 2015-02-17
-
Remove unnecessary AC_SUBST.
- 424. By William Hua on 2015-02-17
-
Use shift operator.
- 425. By William Hua on 2015-02-17
-
Check flags better.
- 426. By William Hua on 2015-02-17
-
Only try initializing Fcitx once.
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:426
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
- 427. By William Hua on 2015-02-18
-
Remove (!) when possible.
- 428. By William Hua on 2015-02-18
-
Use foreach when possible.
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:428
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
- 429. By William Hua on 2015-02-18
-
Remove vapi.
- 430. By William Hua on 2015-02-18
-
Remove unnecessary flag.
We don't really need this flag any more since we decided to hide indicator-keyboard in the session when Fcitx is running since it has its own indicator.
| William Hua (attente) wrote : | # |
Thanks, I think that's all of the necessary changes, including removing the generated vapi files.
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:430
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 431. By William Hua on 2015-02-18
-
Build depend on gir1.2-fcitx-1.0.
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:431
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| Allison Lortie (desrt) wrote : | # |
There's nothing wrong here, really -- just a few nitpicks. I wouldn't let it block the merge, or anything...
| William Hua (attente) wrote : | # |
I filed https:/
| William Hua (attente) wrote : | # |
I don't think we ever scroll input sources by more than 1 at a time, but it's exposed via org.gtk.Actions over D-Bus, so I did that mod just in case. I'll re-write it though to explain why, and fix it so we only need one loop.
- 432. By William Hua on 2015-02-19
-
Don't handle activate signal when the default behaviour will do.
- 433. By William Hua on 2015-02-19
-
Simplify input source cycling.
- 434. By William Hua on 2015-02-19
-
Explain input source cycling logic.
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:434
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| Sebastien Bacher (seb128) wrote : | # |
Thanks, that looks fine, let's get that in a silo on monday, with u-c-c and u-s-d

FAILED: Continuous integration, rev:409 jenkins. qa.ubuntu. com/job/ indicator- keyboard- ci/69/ jenkins. qa.ubuntu. com/job/ indicator- keyboard- utopic- amd64-ci/ 2/console jenkins. qa.ubuntu. com/job/ indicator- keyboard- utopic- armhf-ci/ 2 jenkins. qa.ubuntu. com/job/ indicator- keyboard- utopic- armhf-ci/ 2/artifact/ work/output/ *zip*/output. zip
http://
Executed test runs:
FAILURE: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/indicator- keyboard- ci/69/rebuild
http://