> > > On 15 Feb 2018, at 11:43, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote: > > > >> > >> On Thu, Feb 15, 2018 at 10:56:49AM +0100, Lukáš Hrázký wrote: > >>> On Wed, 2018-02-14 at 22:43 +0100, Christophe de Dinechin wrote: > >>>>> On 14 Feb 2018, at 17:29, Christophe Fergeau <cfergeau@xxxxxxxxxx> > >>>>> wrote: > >>>>> > >>>>> This changes the suggested style to what is currently used in > >>>>> spice-server codebase. This also removes a few sentences which > >>>>> are not really related to how one should format their header guards. > >>>>> > >>>>> Signed-off-by: Christophe Fergeau <cfergeau@xxxxxxxxxx> > >>>>> --- > >>>>> docs/spice_style.txt | 6 ++---- > >>>>> 1 file changed, 2 insertions(+), 4 deletions(-) > >>>>> > >>>>> diff --git a/docs/spice_style.txt b/docs/spice_style.txt > >>>>> index c8a4cff66..bc18b1d9e 100644 > >>>>> --- a/docs/spice_style.txt > >>>>> +++ b/docs/spice_style.txt > >>>>> @@ -365,12 +365,10 @@ Headers should be protected against multiple > >>>>> inclusion using a macro that contai > >>>>> > >>>>> ... > >>>>> > >>>>> -#endif // MY_MODULE_H > >>>>> +#endif /* MY_MODULE_H */ > >>>> > >>>> I had first written it with C style, Frediano suggested C++ style.Both > >>>> are OK. Currently, we have 44 of one and 208 of the other, so the > >>>> existing code base does not imply one or the other. > >>> > >>> FWIW, I see no reason to use /* */ when // is simpler. > >> > >> I don't feel strongly either way as long as it's consistent, and in > >> spice-server case: > >> > >> $ for f in spice/server/*.h; do tail -1 $f; done > >> #endif /* AGENT_MSG_FILTER_H_ */ > >> #endif /* CACHE_ITEM_H_ */ > >> #endif /* CHAR_DEVICE_H_ */ > >> #endif /* COMMON_GRAPHICS_CHANNEL_H_ */ > >> #endif /* CURSOR_CHANNEL_CLIENT_H_ */ > >> #endif /* CURSOR_CHANNEL_H_ */ > >> #endif /* DCC_H_ */ > >> #endif /* DCC_PRIVATE_H_ */ > >> #endif /* DEMARSHALLERS_H_ */ > >> #endif /* DISPATCHER_H_ */ > >> #endif /* DISPLAY_CHANNEL_H_ */ > >> #endif /* DISPLAY_CHANNEL_PRIVATE_H_ */ > >> #endif /* DISPLAY_LIMITS_H_ */ > >> #endif /* GLIB_COMPAT_H_ */ > >> #endif /* GLZ_ENCODER_DICT_H_ */ > >> #endif /* GLZ_ENCODER_H_ */ > >> #endif /* GLZ_ENCODER_PRIV_H_ */ > >> #endif /* IMAGE_CACHE_H_ */ > >> #endif /* IMAGE_ENCODERS_H_ */ > >> #endif /* INPUTS_CHANNEL_CLIENT_H_ */ > >> #endif /* INPUTS_CHANNEL_H_ */ > >> #endif /* JPEG_ENCODER_H_ */ > >> #endif /* LZ4_ENCODER_H_ */ > >> #endif /* MAIN_CHANNEL_CLIENT_H_ */ > >> #endif /* MAIN_CHANNEL_H_ */ > >> #endif /* MAIN_DISPATCHER_H_ */ > >> #endif /* MEMSLOT_H_ */ > >> #endif /* MIGRATION_PROTOCOL_H_ */ > >> #endif /* RED_NET_UTILS_H_ */ > >> #endif /* PIXMAP_CACHE_H_ */ > >> #endif /* RED_CHANNEL_CAPABILITIES_H_ */ > >> #endif /* RED_CHANNEL_CLIENT_H_ */ > >> #endif /* RED_CHANNEL_H_ */ > >> #endif /* RED_CLIENT_H_ */ > >> #endif /* RED_COMMON_H_ */ > >> #endif /* RED_PARSE_QXL_H_ */ > >> > >> #endif /* RED_PIPE_ITEM_H_ */ > >> #endif /* RED_QXL_H_ */ > >> #endif /* RED_RECORD_QXL_H_ */ > >> #endif /* REDS_H_ */ > >> #endif /* REDS_PRIVATE_H_ */ > >> #endif /* RED_STREAM_H_ */ > >> #endif /* RED_WORKER_H_ */ > >> #endif /* SMARTCARD_CHANNEL_CLIENT_H_ */ > >> #endif /* SMART_CARD_H_ */ > >> #endif /* SOUND_H_ */ > >> #endif /* SPICE_AUDIO_H_ */ > >> #endif /* SPICE_BITMAP_UTILS_H_ */ > >> #endif /* SPICE_CHAR_H_ */ > >> #endif /* SPICE_CORE_H_ */ > >> #endif /* SPICE_EXPERIMENTAL_H_ */ > >> #endif /* SPICE_H_ */ > >> #endif /* SPICE_INPUT_H_ */ > >> #endif /* SPICE_MIGRATION_H_ */ > >> #endif /* SPICE_QXL_H_ */ > >> #endif /* SPICE_REPLAY_H_ */ > >> > >> /*** END file-tail ***/ > >> #endif /* SPICE_SERVER_H_ */ > >> #endif /* SPICE_VERSION_H_ */ > >> #endif /* STAT_FILE_H_ */ > >> #endif /* STAT_H_ */ > >> #endif /* STREAM_CHANNEL_H_ */ > >> #endif /* TREE_H_ */ > >> #endif /* UTILS_H_ */ > >> #endif /* VIDEO_ENCODER_H_ */ > >> #endif /* VIDEO_STREAM_H_ */ > >> #endif /* ZLIB_ENCODER_H_ */ > >> > > > > Maybe I helped creating this confusion. > > > > I was looking at the example of the session talking about header guard > > and my though was "let's hope this final comment style is not taken > > literally, the section is about having the guards". > > Personally C style, C++ style or a missing comment (I think in 95% of > > all headers the last #endif is the close of the guard!) is not that > > important. > > From the current style point of view (and taking into account that > > this document is in spice-server) yes, the suggested style for closure > > would be better to have a C style comment with the guard name. > > Sorry for the confusion. > > > > Maybe just a comment stating the end comment style is just an advice? > > I don’t think we want this to be just an advice. The problem Christophe > points out is with existing code. > I think that if we started today, we could all agree on // > > Now, Christophe’s arguments are that > > 1) we should not write guidelines that are inconsistent with existing code. > 2) this is in the server codebase, so we should server rules > > Problem is with 2, really. > > We started updating the guidelines because we wanted to talk about C++ style > in the streaming agent, not the server. > I updated the server guidelines, because that’s historically where the style > guide has been, and the only place where SPICE has one. > > For now, I’d vote for stating that the server guidelines apply to all of the > SPICE code. If we decide that means we should move them elsewhere, that’s > fine with me. > > If that idea is accepted, then Christophe (2) no longer hold, and we can > explicitly state that we accept both // and /* for all comments, including > that one. > > Also, since Christophe was not there at the beginning of the discussion, I > think that the consensus is that guidelines are what we want, not what is > there. If what is there is inconsistent, we’ll slowly fix it over time. > At the beginning there was the void. Then antimatter separate from matter and the big bang happened... maybe too early ? :-D Well, the beginning was a mail from Lukash attempting to define some specific aspect of C++ like method naming and namespace. We were talking on how to integrate these changes and was agreed to put into spice-server doc so we didn't have to change generation and publication of this document. >From that beginning I think the situation went a bit out of control. Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel