[
https://issues.apache.org/jira/browse/THRIFT-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13012554#comment-13012554
]
Christian Lavoie commented on THRIFT-1123:
------------------------------------------
Thanks for the patch -- looks mostly ok. A few comments:
1) Config.h is a bad name -- I'd rather it was called win32_config.h or
platform_specifics.h or something like that.
In particular, on Mac OS X case-insensitive filesystems are common and I'd
rather ./configure doesn't overwrite an existing file under any circumstances.
2) I like how you've done BIND and FLAGS and the rest of those, but they
probably should be defined in their own namespace. Long story short, using ALL
CAPITAL NAMES isn't the prettiest way to identify those as platform specific.
3) I really don't like that Config.h (whatever it ends up being called)
includes that many headers on UNIX. Is there a way to structure this so that
the final .cpp files include only the .h they need, not everything that's
currently in Config.h (whatever it ends up being called)?
The solution to #2 and #3 may be the same, though that's a non-trivial
undertaking.
I've got a few nitpicks here and there, but they're minor compared to what's
above (e.g.: why did you put (std::min) in parentheses in TBufferTransports.cpp
? )
> Patch to compile Thrift server and client for vc++ 9.0 and 10.0
> ---------------------------------------------------------------
>
> Key: THRIFT-1123
> URL: https://issues.apache.org/jira/browse/THRIFT-1123
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Library
> Environment: Windows XP 32bit, vc++ 9.0, 10.0
> Reporter: Dragan Okiljevic
> Priority: Trivial
> Fix For: 0.7
>
> Attachments: thrift_msvc_client_and_server.patch
>
>
> Extension of THRIFT-1031 patch published by James Dickson
> This patch is intended to provide Thrift C/C++ functionality on WIN32
> platforms.
> The implementation is built on top of the patch "Patch to compile Thrift for
> vc++ 9.0 and 10.0" by James Dickson published as THRIFT-1031. I just used
> this code and ported more Thrieft C/C++ to WIN32 and added them to original
> VC projects created in THRIFT-1031.
> I express my gratitude to Mr. Dickson as his post gave me the roadmap how to
> do the additional changes, that I hope, would be useful for the rest of the
> community too.
> Besides client capabilities enabled in THRIFT-1031, the library can now be
> used for building Thrift servers and using concurrency features. The dir/file
> structure from THRIFT-1031 and usage of Config.h header for providing support
> for both WIN32 and *NIX remains.
> The implementation was tested briefly on MSVC2008, MSVC2010 and Ubuntu,
> communicating between C/C++ clients and servers and Java clients and servers.
> As the author needs all of this functionality for one of his projects, the
> testing and debugging will continue.
> Revision 1086435 from March, 28, 2011. was used for development and creation
> of patch, but it should be possible to apply it on current trunk revision as
> long as no changes are made to patched files in trunk.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
