> > > On 15 Feb 2018, at 10:07, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote: > > > > On Wed, Feb 14, 2018 at 10:29:25PM +0100, Christophe de Dinechin wrote: > >> > >> > >>> On 14 Feb 2018, at 17:34, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote: > >>> > >>> On Wed, Feb 14, 2018 at 10:45:56AM -0500, Frediano Ziglio wrote: > >>>>> > >>>>> Shouldn't this go with a Makefile rule? A few lines in the log what > >>>>> this > >>>>> is about, what is the goal for having this file, ... would not hurt. > >>>>> > >>>>> Christophe > >>>>> > >>>> > >>>> I think this file is supposed to just help developers so should not > >>>> be in the Makefile. > >>> > >>> Yes, after reading various threads, it's apparently meant to be used > >>> together with emacs for formatting of small code blocks, it's not usable > >>> on the whole codebase. So a 'make clang-format' rule apparently would > >>> not make sense. > >>> > >>>> I think you mean that the intention should be written in the commit > >>>> message. > >>> > >>> Yes, knowing how it's meant to be used, why we want it in the codebase. > >> > >> Why would we NOT want it in the codebase? > > > > I'm not saying I'm against what this patch is adding. I'm saying > > I'm against a commit adding something without much of a rationale in its > > commit log. > > Fair point. Added that. > > To clarify, right now, I don’t think we can refactor whole files yet, the > code is not ready for it. See > https://gitlab.com/c3d/spice-server/commit/f6297e63f5e785e5fdf7176fd9381d0f465f6c11 > for a review of the server changes, if you are curious. > I think the main reason is that you are attempting to change the current style entirely. For instance we have a section that explicitly state we don't want column alignment. Or see the section about section declaration. > Some of the issues include: > > - Our headers are not yet self-contained, so you get: > In file included from agent-msg-filter.c:25: > In file included from ./red-common.h:37: > In file included from ./spice.h:27: > ./spice-migration.h:47:31: error: unknown type name ‘SpiceServer' > I test quite often and as far as I know they are self contained. In this case spice-migration.h should be included from spice.h and SpiceServer is defined in spice-server.h included from spice.h before spice-migration.h. > - Some comments get munged, notably comments at end of line: > > - Minor things, like a macro where (id) & CONSTANT interpreted as a cast (id) > with address taken and reformatted accordingly > > Christophe If your intention with this file is changing the entire style I would personally avoid it. The subject is "Add .clang-format with defaults matching what's specified in the style guide", from the branch you pointed out looks like is false. Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel