Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(12407)

Issue 5730054: Silence warnings when engine is skipped. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 1 month ago by Daiki Ueno
Modified:
12 years, 1 month ago
Reviewers:
shawn.p.huang
Base URL:
git@github.com:ibus/ibus-m17n.git@master
Visibility:
Public.

Description

Silence warnings when engine is skipped. Also, fix some mistakes. BUG=none TEST=manually with Fedora's default.xml

Patch Set 1 #

Total comments: 4

Patch Set 2 : use EXIT_FAILURE, remove surrounding text check macro, use libtool for libm17ncommon.* #

Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -21 lines) Patch
M src/Makefile.am View 1 4 chunks +6 lines, -6 lines 0 comments Download
M src/engine.c View 1 6 chunks +5 lines, -11 lines 0 comments Download
M src/m17nutil.c View 1 chunk +4 lines, -2 lines 0 comments Download
M src/main.c View 1 1 chunk +3 lines, -2 lines 0 comments Download
M src/setup.c View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 3
Daiki Ueno
On Fedora, we disable several engines by default (i.e. rank < 0). In that setting, ...
12 years, 1 month ago (2012-03-05 06:58:41 UTC) #1
Peng
lgtm http://codereview.appspot.com/5730054/diff/1/src/main.c File src/main.c (right): http://codereview.appspot.com/5730054/diff/1/src/main.c#newcode119 src/main.c:119: exit (1); Maybe you would like to use ...
12 years, 1 month ago (2012-03-05 15:49:42 UTC) #2
Daiki Ueno
12 years, 1 month ago (2012-03-06 01:01:31 UTC) #3
JFTR, uploading a new CL to be committed, that also fixes more minor mistakes:

- remove HAVE_IBUS_ENGINE_GET_SURROUNDING_TEXT macros
- use libtool to link libm17ncommon.a
- mark some internal functions as static

https://codereview.appspot.com/5730054/diff/1/src/main.c
File src/main.c (right):

https://codereview.appspot.com/5730054/diff/1/src/main.c#newcode119
src/main.c:119: exit (1);
On 2012/03/05 15:49:42, Peng wrote:
> Maybe you would like to use EXIT_FAILURE here

Done.

https://codereview.appspot.com/5730054/diff/1/src/main.c#newcode124
src/main.c:124: exit (0);
On 2012/03/05 15:49:42, Peng wrote:
> And EXIT_SUCCESS

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b