broulik added a comment.

  Nice refactoring work! Quick testing suggests a few issues, however. I 
haven't verified if they have been present before or are in KUnitConversion, 
but:
  
  "5 EUR" produces: 4.6095 GBP, 5.467 USD7.8745 CAD (@sitter says this is some 
unrelated bug)
  "5 l/100km" (fuel consumption) doesn't yield any result (@sitter says this 
didn't work before either)
  "5 m²" doesn't yield any results. In fact none of the ² or ³ do
  Units seem to be matching case sensitive now, i.e. "5 Liter" (German locale) 
doesn't yield any results, wheras "5 liter" does

INLINE COMMENTS

> converterrunnertest.cpp:75
> +    QCOMPARE(context.matches().count(), 1);
> +    QCOMPARE(context.matches().first().text(), QStringLiteral("100 
> centimeters (cm)"));
> +}

I think you should change the locale for the unittest to `C` or `en_US` (dunno 
how, needs someone with more test knowledge :D) since those are translated and 
fail when I run the test locally

REPOSITORY
  R114 Plasma Addons

BRANCH
  converter_runner_refactoring (branched from master)

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

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

Reply via email to