[DGD] DGD For MSys, mark 2

[Jared Maddox]

> Date: Thu, 24 Jan 2013 12:38:32 +0100
> From: "Felix A. Croes" <felix at dworkin.nl>
> To: dgd at dworkin.nl
> Subject: Re: [DGD] DGD For MSys, mark 2
> Message-ID: <E1TyL8a-0000Wf-W7 at pattern>
>
> Jared Maddox <absinthdraco at gmail.com> wrote:
>

<snip>

>>  I would, however, like a review.
>
> You got it.
>
> I reviewed the changes on github, using the following URL:
>
>     https://github.com/jam555/dgd/compare/msys
>
> - Review the changes yourself at this URL.  Do you see which ones don't
>   change anything?  Do you see the bug you introduced with LIBS in the
>   main Makefile?
Is it ifeq ($(HOST),MSys) ? Fixed.

Is -LIBS=  -ldl important, or is it included by default? Regardless,
the WIN32 bit has been moved in front of the others, and -LIBS=  -ldl
stuck inside an else associated with it.

>   Tighten up the changes so you respect the existing layout and don't
>   add pointless newlines.
Ah, sorry about that. As you might have guessed, I tend to use a lot
of whitespace.

>   Merge the bit after OS=$(shell uname -s)
>   with the bit before it.
I can move all of the added code after OS=$(shell uname -s) to before
it, but the portion before it is to give an informative error message
if someone tries to run the makefile from a standard Windows command
prompt.

What about the comment above ifeq ($(OS),Windows_NT) ? I worry that
without it, the reason for putting that in such an inconsistent place
may be forgotten.

> - While commentary is good, you go too far with your comments in the
>   Makefile and the README.  Comments should describe the current state
>   of the code, not the entire process of how that state was reached.
>   Of course, I realize that you deliberately overdid it, but there is
>   a place where you can expand in such detail, and where your name and
>   the date will be automatically attached: in the git commit message.
Ok. Though honestly, I was trying just as much to make the commentary
OBNOXIOUS as to provide detail. If I had looked up things like
$(warning ...) earlier in the process then anyone trying to use this
variant of the makefiles would probably think they'd accidentally
found a self-destruct device inside their computer ;) . And this is,
admittedly, one of the reasons why I wasn't going to issue a pull
request yet.

And don't worry, I haven't added a $(warning ...) to replace the comments.
Yet ;) .

> - Most of the required LIBS will be linked in by default.  Is anything
>   beyond  -lws2_32 really needed?
Apparently not, since it compiled & ran. Removed.

> - Tighten up the changes to host/Makefile.  Use a macro for the source
>   directory:
>
>     connect.c: $(PLATFORM_DIR)/connect.c
> 	    cp $(PLATFORM_DIR)/$@ $@
>
>   That way, the same rules can be used for unix and win32.
>
Done.

> Regards,
> Felix Croes
>

Ready for another review.