[Merge] lp:~jonas-drange/ubuntu-settings-components/exposes-notifiable-printer into lp:~phablet-team/ubuntu-settings-components/printer-components

Andrew Hayzen andrew.hayzen at canonical.com
Fri Feb 17 17:53:40 UTC 2017


Review: Disapprove

As discussed I'm not sure if the printing-app needs this as it is able to access the roles from the PrinterModel by using an Instantiator http://pastebin.ubuntu.com/24014281/ . The app could change to have more of a listview structure, but even then I think it would still use the model and roles rather than getPrinter(index).

Also if this was to be done, any logic in PrinterModel::(set)data would need to be moved into Printer as things like isLoaded aren't currently properties on the Printer.

So for now I think it is simpler to leave it as is, and to let the QML use the model and roles.
-- 
https://code.launchpad.net/~jonas-drange/ubuntu-settings-components/exposes-notifiable-printer/+merge/317625
Your team Ubuntu Phablet Team is subscribed to branch lp:~phablet-team/ubuntu-settings-components/printer-components.



More information about the Ubuntu-reviews mailing list