----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7826/#review12997 -----------------------------------------------------------
/proton/trunk/proton-c/CMakeLists.txt <https://reviews.apache.org/r/7826/#comment27976> I don't understand the point of most of these lines: The only ones that I agree with are setting WARNING_FLAGS. Cmake already sets up different compile flags for the different builds, just leave the differences to cmake. If the developer really wants to change the flags they can do it themselves. Any flags that are actually necessary for the build would be in common between the builds so shouldn't be here anyway. /proton/trunk/proton-c/CMakeLists.txt <https://reviews.apache.org/r/7826/#comment27977> Instead of getting rid of this it should read - COMPILE_FLAGS "${WARNINGS_FLAGS} ${OTHER_STUFF}" where OTHER_STUFF (or a better name) is set to -std=c99 for gcc and the flag to compile C as C++ for MSVC. /proton/trunk/proton-c/CMakeLists.txt <https://reviews.apache.org/r/7826/#comment27978> As previous comment /proton/trunk/proton-c/src/protocol.h.py <https://reviews.apache.org/r/7826/#comment27979> I don't like the way the python changes have been implemented: I think it would be much better to have a new separate python script that takes as arguments the additional python path and the rest of the command line to run and then set up the python path and run the next script. This would avoid repeating the same, but slightly different, code twice. And would be extensible to any new pythons script we have to add. And would avoid changing the existing scripts into something less pythonic (IMO) - Andrew Stitcher On Nov. 1, 2012, 2:24 p.m., Kenneth Giusti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7826/ > ----------------------------------------------------------- > > (Updated Nov. 1, 2012, 2:24 p.m.) > > > Review request for qpid, Andrew Stitcher and Cliff Jansen. > > > Description > ------- > > Work In Progress: changes to cmake configuration to support Microsoft Visual > C++ Express 2010 IDE. > > > Diffs > ----- > > /proton/trunk/proton-c/CMakeLists.txt 1404604 > /proton/trunk/proton-c/README 1404604 > /proton/trunk/proton-c/src/codec/encodings.h.py 1404604 > /proton/trunk/proton-c/src/protocol.h.py 1404604 > > Diff: https://reviews.apache.org/r/7826/diff/ > > > Testing > ------- > > > Thanks, > > Kenneth Giusti > >
