Code review comment for lp:~nick-dedekind/unity8/indicators-client-textual-app

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

> Panel/Indicators/client/IndicatorsPage.qml
>
> Please add your name to the "Authors" list in the header
>
> - property alias pageSource : page_loader.source
> + property string pageSource : page_loader.source
>
> Let's take the opportunity to s/page_loader/pageLoader
>
> -----
>
> Panel/Indicators/client/IndicatorsTree.qml
>
> + * Nick Dedekind <<email address hidden>
>
> Missing the closing ">"
>
> + Text {
> + id: all_data
> + color: "white"
> + }
>
>
> s/all_data/allData
>
>
> + Indicators.ModelPrinter {
> + id: printer
> + model: menuModel
> +
> + onSourceChanged: page.refresh()
> + }
>
> + function refresh() {
> + all_data.text = printer.getString();
> + }
>
>
> Couldn't we make it more QMLish by making the string a property of
> ModelPrinter (and name it "text")?
> So code would be simply:
>
> Text {
> id: allData
> color: "white"
> text: printer.text
> }
>
> ----------------
>
> plugins/Unity/Indicators/modelprinter.cpp
>
> + * Copyright (C) 2012 Canonical, Ltd.
>
> s/2012/2013
>
> + QString recurse_string(const QModelIndex& index, int level) const;
>
> s/recurse_string/recurseString
>
> +// Qt
> +#include <QDebug>
>
> s/QDebug/QTextStream
>
> + stream << tabify(level) << roleNames[role] << "." <<
> iter.key() << ": " << iter.value().toString() << endl;
>
> Too long
>
> -------------------
>
> plugins/Unity/Indicators/modelprinter.h
>
> + * Copyright (C) 2012 Canonical, Ltd.
>
> s/2012/2013

Thanks. All done.

« Back to merge proposal