[Merge] lp:~jonas-drange/ubuntu-ui-extras/extended-attributes into lp:~phablet-team/ubuntu-ui-extras/printer-staging

Andrew Hayzen andrew.hayzen at canonical.com
Thu Mar 2 15:37:37 UTC 2017


Review: Needs Information

One inline comment, otherwise looks good now with the changes you've made :-)

1) Any reason you did the return; here and not?

if (name() == printerName) {
  m_stateMessage = text;
}

Diff comments:

> 
> === modified file 'modules/Ubuntu/Components/Extras/Printers/printer/printer.cpp'
> --- modules/Ubuntu/Components/Extras/Printers/printer/printer.cpp	2017-03-01 15:56:50 +0000
> +++ modules/Ubuntu/Components/Extras/Printers/printer/printer.cpp	2017-03-02 15:33:46 +0000
> @@ -315,7 +353,22 @@
>      m_backend = other->m_backend;
>      other->m_backend = tmp;
>  
> -    loadColorModel();
> -    loadPrintQualities();
> -    loadAcceptJobs();
> +    loadAttributes();
> +}
> +
> +void Printer::onPrinterStateChanged(
> +        const QString &text, const QString &printerUri,
> +        const QString &printerName, uint printerState,
> +        const QString &printerStateReason, bool acceptingJobs)
> +{
> +    Q_UNUSED(printerUri);
> +    Q_UNUSED(printerState);
> +    Q_UNUSED(printerStateReason);
> +    Q_UNUSED(acceptingJobs);
> +
> +    if (name() != printerName) {

Any reason you did the return; here and not?

if (name() == printerName) {
  m_stateMessage = text;
}

> +        return;
> +    } else {
> +        m_stateMessage = text;
> +    }
>  }


-- 
https://code.launchpad.net/~jonas-drange/ubuntu-ui-extras/extended-attributes/+merge/318779
Your team Ubuntu Phablet Team is subscribed to branch lp:~phablet-team/ubuntu-ui-extras/printer-staging.



More information about the Ubuntu-reviews mailing list