[Merge] lp:~phablet-team/media-hub/add-logger into lp:media-hub
Jim Hodapp
jim.hodapp at canonical.com
Wed Apr 6 15:27:37 UTC 2016
> I have some concerns with this MP:
>
> * I do not really like moving to the printf way of doing things. Why should be
> drop type safeness? It would be better to use the boost logger more "natively"
> with "<<" operators, changing the wrapper class to something similar to what
> Qt does (qDebug() << message, etc.). Even more, using printf way in the end we
> need to resort to using stringstream in several places where it was not needed
> before.
Boost still maintains complete type safety with using the printf style. See the Boost::Log docs about this if you have any questions. This private Boost::Log interface implementation comes from tvoss and we're starting to standardize on it. Not sure why it doesn't use streams but that's outside anything I can really control without rewriting the private logger interface.
>
> * I still see cout and cerr being used in many places. But please do not
> change that yet until we have an agreement on how the class Logger should be
> used.
There are some that will need to remain, but otherwise I've made sure that all superfluous ones have now been removed.
>
> * I do not think we should introduce functions in the Utils namespace that we
> do not use. Specially taking into account that I would prefer to avoid using
> the only one that seems to be used, Sprintf :)
Fixed
>
> * Line sizes in new files are > 100 in many cases
I tried not to do that, any specific ones?
>
> There are also a couple of inline comments.
--
https://code.launchpad.net/~phablet-team/media-hub/add-logger/+merge/291010
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub.
More information about the Ubuntu-reviews
mailing list