kdereview: bodega

[Ben Cooksley]

On Sun, Nov 4, 2012 at 1:49 AM, Albert Astals Cid <aacid at kde.org> wrote:
> El Dimecres, 24 d'octubre de 2012, a les 22:07:10, Aaron J. Seigo va escriure:
>> hi :)
>>
>> (x-posting between core-devel and -devel as this is (hopefully) of general
>> interest and because we also are supposed to announce new kdereview
>> modules.)
>>
>> So ... what is this "Bodega" thing I speak of?
>>
>> <snip>
>>
>> If you are interested, please check out the repositories which are now in
>> kdereview: kde:bodega-client and kde:bodega-server and/or ask questions
>> which I'll do my best to answer

Hi Aaron,

>
> Won't have time to give it a use test, but some things grep and friends told
> me
>
> ** bodega-client **
>
> i18n is messed up
>  * the primavera/ folder has i18n but no Messages.sh
>  * the ./activeclient/src/Messages.sh extracts to a catalog named active-
> addons that doesn't seem to be used
>  * You have a ki18n(""), not sure what you expect people to translate there
> ;-)
>
>
> Both primavera and activeclient have this option
>   options.add("opengl", ki18n("use a QGLWidget for the viewport"));
>   options.add("opengl", ki18n("use a QGLWidget for the viewport"));
> which to be honest not sure the value they have to the end user
>
>
> There's a few foreach missing const &
>
>
> The foreach loop in qScriptValueFromTags is unnecessary slow (doing keys +
> values) (it's ok if the length of tags is never high)
>
>
>
>
> ** bodega-server **
>
> assetimporters/projectgutenberg/src/lcc.cpp has no copyright in the header
>
> assetimporters/projectgutenberg/src/lcc.cpp has QObject::tr calls, what's the
> deal with those, are you expecting a translation? If so you'll need a
> Message.sh, if not, do we need them?
>
> There's a few foreach missing const &

What is the status of the review of bodega-client and bodega-server?
It seems that the 2 weeks review period has since passed, but there
are unresolved review problems. Can you please take a look?

>
>
> Cheers,
>   Albert

Thanks,
Ben