|
From: | Priyesh kumar |
Subject: | Re: Logging Library-GSOC |
Date: | Sat, 29 Aug 2020 15:13:53 +0530 |
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
[Prev in Thread] | Current Thread | [Next in Thread] |