Re: New patch for user defined out of date.

[Paul Smith]

On Mon, 2007-07-02 at 21:22 +0200, Ramón García wrote:
> Sorry, forgot to attach the patch.

Hi Ramon; I'm looking at the patch and I'll have more comments later I
expect.

Just a quick note: although somewhat pedantic it's important to have a
consistent coding style throughout the source code.  The style for GNU
make is the GNU C style (if you use Emacs cc-mode); check the rest of
the code for examples of brace placement, indentation rules, space
between function and open paren, etc.  It may not be the style you'd
choose (and indeed there are parts to it that I have to continuously
remind myself about, even today) but in the end a consistent style makes
the code more readable and understandable.

Also, files such as Makefile.in are generated (during the distribution
creation phase) and not checked in, so it's not useful to include them
in patches.  Anyone who wants to apply the patch and build will need all
the autotooling installed anyway, so patching Makefile.in won't be
necessary.

Third, please remember that all code changes must have unit tests
written to exercise their different features and those should be part of
the checkin.  For a project this size it might not make as much sense
(or it might!) but I tend to steal a page from Extreme Programming and
write the tests FIRST, then write the code--when the code runs all the
tests correctly then I'm done.

Finally, I think we should have a different name for the user_dep.*
files.  I don't really know what "user defined dependencies" means; to
me it doesn't describe what this is.  We aren't defining new
dependencies; we're defining new ways of determining up-to-date-ness.  I
guess what I was thinking was we could extract the entirety of the
"out-of-date" detection, which is basically all the stuff dealing with
mtime (file_mtime, comparisons, etc.), out of remake.c into a separate
file with a well-defined interface.  That file could be named something
appropriate.

To me, almost more important than whatever algorithm is implemented is
the cleanup/restructuring that's done to abstract the algorithm from the
rest of the code.  This will allow us to easily and safely make changes
to the algorithm in the future, should that become necessary.

-- 
-------------------------------------------------------------------------------
 Paul D. Smith <address@hidden>          Find some GNU make tips at:
 http://www.gnu.org                      http://make.paulandlesley.org
 "Please remain calm...I may be mad, but I am a professional." --Mad Scientist