Hi Alexander, > It doesn't seem right to apply different rules to headers that are > exported and not exported. Are you going to rewrite the includes > if/when the class ImageGrabber becomes a public library? (E.g. when > someone wants to incorporate a screen grabber into his application.)
Nothing in KScreenGenie will ever become a public library. If someone wants a screen grabber in his application, the correct way to do it is by interfacing with KSG via DBus (which, btw, it lacks currently, but will have in the future). In a public library, things are much more strict - no extra includes, dpointers etc., filenames are lowercase, and so on. It doesn't have to be so strict here. > You said "I can see exactly which components a single cpp file depends > on my looking at the headers". If you follow the recommendation in the > links [1,2], all the dependent components will be included in the .cpp > file, so you can see exactly what you want there. Any other issue with > it? No, some of them stay over in the header file. "Everything in one place", for a small application such as this, which is not a public library, takes precedence over minorly increased build times. > And finally, why using distinct coding style while there's established > Kdelibs coding style? Is your coding style documented somewhere? If > not, then I'm sure KSG will become a mix of different coding styles in > a while because people don't know what coding style to follow. It's kdelibs, with two exceptions: Cumulative headers, and all the access modifiers (public, private etc) stay aligned with the function definitions in the class (i.e., 1 level indented). BTW I've gone ahead and reverted the commit and parts of your other commits. The only thing that was required, was to make sure in every place X11ImageGrabber is included, that it should be included only if XCB_FOUND is defined. I realise that you have been a KDE developer for far longer than I have, but I would still like to refer you to the KDE Commit Policy [1], sections 1.8 and 1.13. Personally I make it a rule to use Review Board if I submit even one-liners, because I believe the maintainer should be made aware, in advance, of every single change other devs are making to his code, and be given a chance to approve/reject that. However, I do not require that for KScreenGenie (in fact, quite a few other devs have made minor changes, both to code and to the docs, and I have nothing but sincere thanks for them for fixing these issues). That said, having to go through the commitdiffs for *four* unannounced commits, two of which were major, to find the *one* location where the real fix was and isolating that, is a huge waste of time which I could have otherwise used to make important pending fixes to the UI and program behaviour. -- Boudhayan
