freetype-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [patch] Simplify ftconfig.h


From: David Turner
Subject: Re: [patch] Simplify ftconfig.h
Date: Wed, 8 Jul 2020 13:32:00 +0200

Here's a patch to add a .clang-format style file to the directory. Note that this doesn't reformat the sources, but see the instructions in the commit message to do that.
I dinf the results quite close to what we have. Please take a look and let me know your opinon on the results.

Apart from that, there are several ways to enforce rules, for example using git hooks that will run cheks before a submit or a push.
Other options are using a code review tool so that each patch is vetted properly before being submitted, but that's more related to the discussion about moving to gitlab/github.

Le mar. 7 juil. 2020 à 07:09, Werner LEMBERG <wl@gnu.org> a écrit :

>> However, I've already invested a lot of time in writing ChangeLog
>> entries...
>
> Would you consider dropping the ChangeLog entries.

Basically, I don't object to that.  However, experience has shown that
people tend to write very sloppy git commit messages in general.  The
ChangeLog format enforces you to describe changes in a standardized
way that I consider very helpful.

> It is far simpler to rely on the git history for this, and just
> enforcing that committer provide meaningful commit messages should
> be enough?

If we can do what Emacs does – namely git commit messages in ChangeLog
style – I'm all for dropping direct ChangeLog entries.

> Also, whenever you reformat some of the patches that are sent to
> you, the original author has to resolve the conflicts manually, or
> drop its own branch, which is not always practical (e.g. when there
> are other work-in-progress commits/branches on top of the one that
> was originally submitted).

This is admittedly a problem.

> Would you consider using clang-format to automate the formatting
> task? I think we can get pretty close to the FreeType formatting
> standard with a Clang style sheet.  It won't be 100% the same, but
> close enough to avoid repetitive work.

If you can manage to write that, please do!  Automatic formatting has
definitely benefits.  Honestly, I suggest that we get completely rid
of the special FreeType formatting.  For consistency I'm enforcing it
on all pieces of code, but virtually all contributors have
difficulties to follow the (unwritten) rules.  Something like

  int main(int argc,
           char **argv)
  {
    FT_Error error;
    FT_Library library;
    FT_Face face;
    int i;


    error = FT_Init_FreeType(&library);
    if (error)
    {
      foobar();
      die("FT_Init_FreeType failed");
    }

    if (argc != 2)
      die("no font file argument given");

    ...

looks nice IMHO – and is quite standard, AFAICS.  Of course,
vertical alignment like

    FT_Error   error;
    FT_Library library;
    FT_Face    face;
    int        i;

is quite nice, too, but I doubt that it can be really handled
automatically.

Whatever you come up with, please check whether our API documentation
can be built correctly.  Additionally, such a change should be
introduced right after a release.

> here's another patch that tries to fix this.  Sorry about that.
> Also Ben Wagner noticed me that some third-party code is using
> FT_UNUSED(), so I've moved it back to public-macros.h to avoid
> breaking this.

Thanks, applied (with minor additions).


    Werner

Attachment: 0001-build-Add-.clang-format-file.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]