Merge lp:~jfi/dee/LP988443 into lp:dee

Proposed by Jean-Philippe Orsini on 2012-07-16
Status: Merged
Approved by: Michal Hruby on 2012-07-17
Approved revision: 373
Merged at revision: 372
Proposed branch: lp:~jfi/dee/LP988443
Merge into: lp:dee
Diff against target: 38 lines (+1/-5)
3 files modified
src/dee-filter.h (+0/-2)
src/dee-model-reader.h (+0/-2)
src/dee-transaction.h (+1/-1)
To merge this branch: bzr merge lp:~jfi/dee/LP988443
Reviewer Review Type Date Requested Status
Michal Hruby (community) 2012-07-16 Approve on 2012-07-17
Jean-Philippe Orsini (community) Resubmit on 2012-07-16
Review via email:

This proposal supersedes a proposal from 2012-07-15.

Commit message

Make sure public dee headers compile with -pedantic option

Description of the change

It fixes:

dee.h is including dee-filter-model.h, dee-filter.h, and dee-model-reader.h.

The 3 header files are defining the typedef DeeFilter.

It implies compilation errors with clang and warnings with 'gcc -pedantic' when compiling a code including dee.h. dee cannot be compiled with clang for this reason too.

1/ According to comment in dee-filter.h and dee-model-reader.h, only dee.h can be included.

2/ dee-filter.h is included by dee-filter.c and dee-filter-model.c but both are also including dee-filter-model.h, so DeeFilter will be defined. dee-filter-model.h is even included by dee-filter.h itself.

3/ dee-model-reader.h is included by dee-index.c and dee-model-reader.c but both are not using DeeFilter

4/ I have verified that compilation working fine after the modification of this branch with both clang and gcc

So, my conclusion is that the typedef can safely be removed from dee-filter.h and dee-model-reader.h or did I miss something?

There also an erronous comma at end of enumerator list which has been removed.

It can be tested by compiling the following code:
#include <dee.h>

int main(int argc, char **argv)
 return 0;

with the command lines:
 - gcc `pkg-config --cflags dee-1.0` -pedantic file.c
 - clang `pkg-config --cflags dee-1.0` file.c

To post a comment you must log in.
Michal Hruby (mhr3) wrote : Posted in a previous version of this proposal

Hi, thanks for contribution! I tried compiling the test you attached, and I'm also getting:
/usr/include/dee-1.0/dee-transaction.h:104:38: warning: comma at end of enumerator list [-pedantic]

Could you please fix that in this branch as well, to make the public headers pedantic-proof?

review: Needs Fixing
Jean-Philippe Orsini (jfi) wrote :

Hello Michal, I have removed the comma and 'request another review' (hope that's the appropriate way to continue the review workflow).

review: Resubmit
Michal Hruby (mhr3) wrote :

No need to request another review unless one of reviewers ask, launchpad updates the diff automatically. Anyway it looks good now, thanks a lot!

review: Approve
Unity Merger (unity-merger) wrote :

No commit message specified.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/dee-filter.h'
2--- src/dee-filter.h 2012-01-09 14:33:36 +0000
3+++ src/dee-filter.h 2012-07-16 18:23:46 +0000
4@@ -30,8 +30,6 @@
8-typedef struct _DeeFilter DeeFilter;
10 /**
11 * DeeFilterMapFunc:
12 * @orig_model: The model containing the original data to filter
14=== modified file 'src/dee-model-reader.h'
15--- src/dee-model-reader.h 2011-12-15 11:28:16 +0000
16+++ src/dee-model-reader.h 2012-07-16 18:23:46 +0000
17@@ -30,8 +30,6 @@
21-typedef struct _DeeFilter DeeFilter;
23 /**
24 * DeeModelReaderFunc:
25 * @model: The model being indexed
27=== modified file 'src/dee-transaction.h'
28--- src/dee-transaction.h 2011-12-15 10:49:06 +0000
29+++ src/dee-transaction.h 2012-07-16 18:23:46 +0000
30@@ -101,7 +101,7 @@
31 */
32 typedef enum {
36 } DeeTransactionError;
38 /**


People subscribed via source and target branches