On Wed, 2018-01-24 at 12:05 -0500, Frediano Ziglio wrote: > > > > Hello, > > > > my first patch set, feel free to nitpick :) > > > > Just an assortment of small improvements, there will be a followup > > email for > > some general coding style and some more use of C++ standard > > library. > > > > Lukáš Hrázký (4): > > use spaces around '=' at the end of methods declaration > > replace tabs with 8 spaces > > use std::string where it's easy to replace > > better error message when opening streaming device > > > > include/spice-streaming-agent/frame-capture.hpp | 14 +++---- > > include/spice-streaming-agent/plugin.hpp | 14 +++---- > > src/concrete-agent.cpp | 10 ++--- > > src/concrete-agent.hpp | 4 +- > > src/spice-streaming-agent.cpp | 49 > > +++++++++++++------------ > > 5 files changed, 46 insertions(+), 45 deletions(-) > > > > Patches looks good. > > I don't understand why I have 2 4/4 on my e-mail but they looks the > same. Yeah, was wondering about that too, until... see below. > About 1/4 I would definitively add a style document so next people > won't use another spacing but this is a follow up. Ok, will follow up. > About commit messages I would extend a bit: > - 3/4 I would explain why you want to use std::string instead of C > pointer (style/memory management); Will do. No memory management advantages here, just a good practice in C++. > - 4/4 I would rephrase the error management is not changed, you > improved the error message. I actually realized this while sending the patches and that's why there are two 4/4 patches, each has a slightly different name/commit message. I didn't notice there ended up being two files before sending. v2 incoming. Lukas _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel