freetype-devel
[Top][All Lists]
Advanced

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

Re: [patch] Simplify ftconfig.h


From: Werner LEMBERG
Subject: Re: [patch] Simplify ftconfig.h
Date: Tue, 07 Jul 2020 07:09:36 +0200 (CEST)

>> 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

reply via email to

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