Thanks! On Friday 30 September 2016 10:08:27 David Faure wrote: > On samedi 10 septembre 2016 17:47:00 CEST Volker Krause wrote: > > Hi, > > > > please review KF5::SyntaxHighlighting (syntax-highlighting in Git) for > > becoming a framework :) > > Looks good. I found a few things though. > > I see that a Jenkins job exists, but it's missing from this view > https://build.kde.org/view/Frameworks%20kf5-qt5/
Is that something I can do directly, or is this done via a ticket for the CI? > Fully missing API docs in : > DefinitionDownloader > HtmlHighlighter Both are not installed, they are only exported for the CLI tool. > Also seen in HtmlHighlighter: > void setOutputFile(FILE *fileHandle); > Shouldn't this use QFile or QIODevice instead? > Or QTextStream, looking at the implementation. Possible, the reason for this was supporting output to stdout in the CLI tool. But as mentioned above, this is not installed/public API anway. > I'm also concerned that this class has direct member vars rather than a d > pointer. > > SyntaxHighligher: missing API docs on methods; missing d pointer. The d pointer is in the base class already, so I don't think we need another one here, do we? > Once these issues are solved you can set the release flag to true in the > yaml file, from my point of view. Regards, Volker
signature.asc
Description: This is a digitally signed message part.
