freetype-devel
[Top][All Lists]
Advanced

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

Re: Logging Library-GSOC


From: Priyesh kumar
Subject: Re: Logging Library-GSOC
Date: Tue, 25 Aug 2020 23:29:46 +0530

Hi,
> Create a new branch that presents your code in an ordered fashion,
>    this is, using not-too-large, concise, well documented commits
>    that makes it easy to understand how your project works.  Have a
>    look how Anuj is doing this to get an idea.
>    While doing so, please check the code and documentation
>    formatting.
>    - Two spaces after a full stop.
>    - Source code files should always have a newline character at the
>      very end (again an editor setting).
>    - Avoid trailing whitespace (this should be an editor setting)
>    - Don't precede tab characters with spaces (again an editor
>      setting).


Is there a git way to do it or do I have to manually order changes and do commits?

> I wonder whether it is necessary to have a separate `dlg.vcxproj`
>  file (and friends).  What about folding its contents into
>  `freetype.vcxproj`?  The `dlg` stuff gets completely integrated into
>  FreeType and will be always linked statically...
>  Am I missing something?
I have made a separate project for dlg, as for the release builds(Release Static and Release)
it would be easy to not compile the dlg's code just by simply skipping the dlg project and
building only the FreeType project.

> * I expect that
>    FT_LOGGING_FILE=/dev/stderr
>  works to emit data to stderr.  It should probably be documented.
Yes, it works, will update this in documentation...

> Please keep `devel/ftoption.h` in sync with
>   `include/freetype/config/ftoption.h`.  Right now, the former lacks
>   `FT_LOGGING`.  Note that the devel version should have FT_LOGGING
>   activated by default, while the standard one should not

Ok.

> Have you checked that `make refdoc` builds ok, and that your
>  `debugging_apis` stuff is properly displayed as HTML in the API
>  documentation?

Yes, it is working properly.

> For function definitions (and especially API functions), always use
>   one argument per line.  This is, don't write
>    int foo( char  bar, void  *baz )
>   but
>    int
>    foo( char   bar,
>         void  *baz )
Ok.

> Have you ensured that the `dlg` files are compiled into the library
>  only if `FT_LOGGING` is defined?  As far as I can see, this is not
>  the case yet[...]

Ok, I will use the dlgwrap.c method, for this we also have to include 
file `freetype.h' right?

> In `toplevel.mk`, change
>    # For builds that don't use `./configure' script ...
>  to
>    # For builds directly from the git repository .
Updated, thanks.

On Tue, Aug 25, 2020 at 11:45 AM Werner LEMBERG <wl@gnu.org> wrote:

Hello Priyesh,


> I have updated the FreeType's code in accordance with the recent
> changes in the dlg library and again tested the code on
> Linux(Autotools), Windows(Visual Studios), and on OS X(Autotools).

thanks!

> I wanted to ask that is there something else which I have to take
> care of?  or should I proceed towards writing the final report?

In addition to the final report you should do two things.

(1) Rebase your code to current 'master'.

(2) Create a new branch that presents your code in an ordered fashion,
    this is, using not-too-large, concise, well documented commits
    that makes it easy to understand how your project works.  Have a
    look how Anuj is doing this to get an idea.

    While doing so, please check the code and documentation
    formatting.

    - Two spaces after a full stop.
    - Source code files should always have a newline character at the
      very end (again an editor setting).
    - Avoid trailing whitespace (this should be an editor setting)
    - Don't precede tab characters with spaces (again an editor
      setting).

    If you do, for example,

      git diff master

    on the command line then trailing whitespace is marked in red, and
    right now you will see *a lot* of red boxes :-)


Some comments regarding the code.

* I wonder whether it is necessary to have a separate `dlg.vcxproj`
  file (and friends).  What about folding its contents into
  `freetype.vcxproj`?  The `dlg` stuff gets completely integrated into
  FreeType and will be always linked statically...

  Am I missing something?

* I expect that

    FT_LOGGING_FILE=/dev/stderr

  works to emit data to stderr.  It should probably be documented.

* Please keep `devel/ftoption.h` in sync with
  `include/freetype/config/ftoption.h`.  Right now, the former lacks
  `FT_LOGGING`.  Note that the devel version should have FT_LOGGING
  activated by default, while the standard one should not.

* Have you checked that `make refdoc` builds ok, and that your
  `debugging_apis` stuff is properly displayed as HTML in the API
  documentation?

* For function definitions (and especially API functions), always use
  one argument per line.  This is, don't write

    int foo( char  bar, void  *baz )

  but

    int
    foo( char   bar,
         void  *baz )

* Have you ensured that the `dlg` files are compiled into the library
  only if `FT_LOGGING` is defined?  As far as I can see, this is not
  the case yet.

  Two possible solutions

  - Adjust the copying process from the 'dlg' sub-repository to
    `src/dlg`, for example do

      echo "#ifdef FT_LOGGING"                             > dlg.c
      echo                                                >> dlg.c
      cat submodules/dlg/src/dlg/dlg.c                    >> dlg.c
      echo                                                >> dlg.c
      echo "#else /* !FT_LOGGING */"                      >> dlg.c
      echo "/* ANSI C doesn't like empty source files */" >> dlg.c
      echo "  typedef int  _dlg_dummy;"                   >> dlg.c
      echo                                                >> dlg.c
      echo "#endif /* !FT_LOGGING */"                     >> dlg.c

    You might use the `sed` program instead of `echo` and `cat`.

  - Create a wrapper file (say, `dlgwrap.c`) that contains

      #ifdef FT_LOGGING
      #  include "dlg.c"
      #else
        /* ANSI C doesn't like empty source files */
        typedef int  _dlg_dummy;
      #endif

    and compile this instead.

* In `toplevel.mk`, change

    # For builds that don't use `./configure' script ...

  to

    # For builds directly from the git repository ...


   Werner

reply via email to

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