> On 23 Feb 2018, at 10:58, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
>
> On Fri, Feb 23, 2018 at 08:18:52AM +0100, Christophe de Dinechin wrote:
>>
>>> On Feb 23, 2018, at 8:07 AM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
>>>
>>> From: Christophe de Dinechin <dinechin@xxxxxxxxxx>
>>>
>>> Signed-off-by: Christophe de Dinechin <dinechin@xxxxxxxxxx>
>>> ---
>>> Change since v3:
>>> - change enum syntax.
>>>
>>> Change since v2:
>>> - rebased.
>>>
>>> Change since v1:
>>> - do not clash with possible short 'b' option.
>>> ---
>>> src/spice-streaming-agent.cpp | 13 +++++++++----
>>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
>>> index 4b14b6f..494cf8e 100644
>>> --- a/src/spice-streaming-agent.cpp
>>> +++ b/src/spice-streaming-agent.cpp
>>> @@ -58,9 +58,9 @@ struct SpiceStreamDataMessage
>>>
>>> static bool streaming_requested = false;
>>> static bool quit_requested = false;
>>> +static bool log_binary = false;
>>> static std::set<SpiceVideoCodecType> client_codecs;
>>> static int streamfd = -1;
>>> -static int log_binary = 0;
>>> static std::mutex stream_mtx;
>>>
>>> static int have_something_to_read(int timeout)
>>> @@ -451,11 +451,13 @@ int main(int argc, char* argv[])
>>> int logmask = LOG_UPTO(LOG_WARNING);
>>> const char *pluginsdir = PLUGINSDIR;
>>> enum {
>>> - OPT_PLUGINS_DIR = UCHAR_MAX+1
>>> + OPT_first = UCHAR_MAX,
>>> + OPT_PLUGINS_DIR,
>>> + OPT_LOG_BINARY,
>>> };
>>
>>> - struct option long_options[] = {
>>> + static const struct option long_options[] = {
>>> { "plugins-dir", required_argument, NULL, OPT_PLUGINS_DIR},
>>> - { "log-binary", no_argument, &log_binary, 1},
>>> + { "log-binary", no_argument, NULL, OPT_LOG_BINARY},
>>> { "help", no_argument, NULL, 'h'},
>>> { 0, 0, 0, 0}
>>> };
>>> @@ -486,6 +488,9 @@ int main(int argc, char* argv[])
>>> agent.AddOption(optarg, p);
>>> break;
>>> }
>>> + case OPT_LOG_BINARY:
>>> + log_binary = true;
>>> + break;
>>> case 'l':
>>> log_filename = optarg;
>>> break;
>>
>> Was about to add
>>
>> Acked-by: Christophe de Dinechin <dinechin@xxxxxxxxxx>
>>
>> then realized it was signed-off by me as well, which makes it weird ;-)
>
> Just realized by reading the commit log and what I remember from the
> reviews, I only have a very vague idea regarding what this is about.
> commit log needs improving... (I really should start nack'ing
> patches with only a short log without even reading them…)
Well, initially, it was just changing log_binary from int to bool.
But then it diverged because of the option processing aspect. I had used short option b, Frediano did not like it (he still did not convince me ;-), so then he replaced that with an enum for long options, and then Snir made another commit with a long option and there was a conflict risk with two options being initialized to UCHAR_MAX+1, so I asked for OPT_first, and now we end up with something that deserves a long commit log.
The suggestion about nacking commits that don’t have a long log seems a bit extreme for one-line cleanups.
>
> Christophe
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel