> Andrei
>
> I took a look at curlpp.sln and curlpp.vcproj files you modified.
>
> I see you changed names of directories and output files from hard coded ones to a new form using macros like "$(ConfigurationName)". That was something I was going to make before I saw you already did this :)
>
> You also changed hard coded path to libcurl to one using LIBCURL_PATH macro. It's nice.
> Maybe we should do the same for curlpp and change accordingly this information in README.win32
> "The makefile requires the libcurlpp.lib file to be in the examples/directory; after building curlpp copy it here." ?
>
> You divided full path to libcurl from AdditionalDependencies to a filename (AdditionalDependencies) and path (AdditionalLibraryDirectories).
> I had to learn this by my mistakes... (I'm new to VC).
>
> What I would propose is to make IntermediateDirectory different then OutputDirectory. It may be a subdirectory of OutputDirectory. So instead of
> IntermediateDirectory="$(ConfigurationName)"
> I would write
> IntermediateDirectory="$(ConfigurationName)\intermediate"
>

Well, splitting intermediate and target stuff is mostly a matter of
taste. But if I do so, I'd rather leave intermediate stuff where it is
now and make a separate dir (say, 'lib') for the target stuff.

> In curlpp.vcproj;
>
> there is this line
> InheritedPropertySheets="$(VCInstallDir)VCProjectDefaults\UpgradeFromVC71.vsprops"
>
> I'm not sure if we should keep this.
> What do you think?

This can be removed.

>
> there is this setting
> Detect64BitPortabilityProblems="true"
>
> whereas I get warning saying this compiler switch is depreciated and will be removed. We should change it to false.

Agree, starting from MSVS 2008 this switch is deprecated. As I said, I
never tested curlpp with VC 9.

>
> warning level is set to 3.
> I set it to 4 and got warning C4512 (assignment operator could not be generated) for the following classes
> FormPart, FormParts::File, FormParts::Content.
> I think we should change warning level to the highest one (4) and of course fix these classes.
>
> I get warning C4005: 'PACKAGE_NAME' : macro redefinition but I see you fixed SList.cpp include list. Still includes in this file don't look ok with this double inclusion of global.h - one with guard and one without. I think we should change this.
>
> I get warning C4706: assignment within conditional expression in line 107 of multi.cpp
> while ((msg = curl_multi_info_read(mMultiHandle, &msgsInQueue)))
> This should be rewritten to
> while ((msg = curl_multi_info_read(mMultiHandle, &msgsInQueue)) !=NULL)

The usual practice is to use default warning level for development. High
warning levels are useful compile with from time to time but this should
not be used as default.

Whether you should fix level 4 warnings depends on the concrete case and
on your own paranoic level ;).

>
> I don't know why you defined this
> #define BUILDING_CURLPP 1
> in config.win32.h? This should only be defined when building curlpp. Did I missed something?

Did I ?

>
> We should rename curlpp.sln and curlpp.vcproj files to curlpp.vc8.sln and curlpp.vc8.vcproj and add curlpp.vc9.sln and curlpp.vc9.vcproj files when they'll be ready.

Yes and no.
Yes - it is easier for the MSVS library user
No - because the developers will have to maintain project file for each
MSVS version which are 99% the same.
I suggest the trade-off solution: until the project/solution files are
back compatible, we keep the lowest version and let the Visual Studio
to convert them if necessary to a higher version. I dunno if VC8
projects are convertible to VC9 but if yes, I'd leave it in 8.

>
> The last but not the least we should add support for compiling with OpenSSL.
>
I did not look at it, but I think this should be handled on libcurl level...

> I think instead of writing all these I should have changed the code and post diffs :)
> cURLpp is a fine and very usefull library and it deserves more time spent on making it even better.

Also please note I just committed a change to curlpp: OptionSetter.cpp
is now included in MSVC project.

And one more thing: *currently* I am not using curlpp in any project. I
bumped into curlpp some time ago when I was looking for C++ HTTP client
library for Windows. That time I just played with curlpp for a while,
did some smoky tests, made sure it satisfy my requirements (proxy
authorization support) and did some corrections. I general I was
satisfied with the library, and it is likely we will use it in our
project but it will happen not earlier than beg. 2009. When it happens,
it will be a real case for curlpp, hope I will not regret my choice ;)
_______________________________________________
cURLpp mailing list
cURLpp_at_rrette.com
http://www.rrette.com/mailman/listinfo/curlpp
Received on 2008-11-11