Re: Logging Library-GSOC

[Priyesh kumar]

Hi Werner,

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

So, I have updated the code in `GSoC-2020-priyesh' branch such that 

there are no separate project files for building dlg, in this branch 

I have used the `dlgwrap.c' method, now dlg is compiled along with FreeType

only for `Debug' and `Debug Release' configurations on Windows if 

`FT_LOGGING' macro is defined.

I have also pushed all the changes in this new branch,

Please take a look at that and let me know if there are any concerns.

Thanks,

Priyesh

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