sitter added a comment.

  Please make a cpps for you headers and move the function implementations 
there.
  
  +1 to regex

INLINE COMMENTS

> converter_utilities.h:23
> +
> +double stringToDouble(const QStringRef &value, bool *ok, const QLocale 
> &locale)
> +{

Perhaps I am missing something but it seems to me that all the functions in 
here really should be statics inside the runner.cpp, don't you think?

> converterrunner.cpp:64
>  {
> -    const QString term = context.query();
> -    if (term.size() < 2) {
> +    const QString &term = context.query();
> +    if (term.size() < 2 || !context.isValid()) {

Is there a particular reason you hold it as a reference? (also applies to a 
bunch of QStrings and QLists below). We generally do not hold Qt types as 
references unless there is a reason to I think.

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D27166

To: alex, broulik, ngraham, #plasma
Cc: sitter, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

Reply via email to