[Spice-devel] [PATCH spice-streaming-agent 0/4] Minor improvements and cosmetics

[Lukáš Hrázký]

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