Code review comment for lp:~jobinau/drizzle/mingwport

Revision history for this message
Monty Taylor (mordred) wrote :

There are a couple of fixes that need to happen before this is merged, although I'm happy to see it working!

The #include "config.h" line needs to be removed from my_sys.h. We do not put that in headers - instead, it should be the first thing included by the .cc file. (Which should already be happening) If this is breaking, please let me know how and I'll help you find a solution.

Also, my_sys.h should really not be chmod +x - there is no need for it to be executable.

#ifdef HAVE_SYS_MMAN_H addition looks great though.

review: Needs Fixing

« Back to merge proposal