Re: Review of VS 2010 support patches - Mailing list pgsql-hackers

Andrew Dunstan wrote:
>
> Some minor nitpicks:
>
> Do we really need to create all those VSnnnnProject.pm and 
> VSnnnnSolution.pm files? They are all always included anyway. Why not 
> just stash all the packages in Solution.pm and Project.pm? 
We certainly don't *need* them.
Having different files separates the tasks of generating different 
target file formats into different source files. In my opinion this 
makes it easier to find the code that is actually generating the files 
that get used in a specific build environment.
While the VSnnnnSolution.pm and VC200nProject.pm files are indeed not 
much more than stubs that could eventually be extended in future (and 
probably never will) VC2010Project.pm contains the whole code for 
generating the new file format which would significantly bloat up the 
code in Project.pm that currently contains the common code for 
generating the old file formats.

Anyhow - this is just my opinion and my intention is to help improving 
the Windows build process and not forcing my design into the project.

> Also, instead of doing this in Mkvcbuild.pm:
>
>    my $vsVersion = VSObjectFactory::DetermineVisualStudioVersion();
>    $solution = VSObjectFactory::CreateSolution($vsVersion, $config);
>
> why not just add "use VSObjectFactory;" at the top of the file and 
> import these into the current namespace, just as we do for pretty much 
> everything else?

Yes - my way (singleton, clean namespace) is probably overengineering in 
this context.

>
> There are some stylistic things that aren't the way I usually do 
> things (use of named instead of anonymous file handles, use of 
> heredocs instead of qq{} style quotes) and that I would prefer done 
> differently, but those are more matters of taste than substance. 
Please go ahead and change it to whatever style you prefer. There is 
certainly more than one way to style it ;-)

> I also generally dislike composing XML by non-formal means, as it can 
> be quite error prone and often leads to errors in unforeseen corner 
> cases. But in this case we certainly don't want to impose an extra 
> requirement on some perl XML module, and it would make this code 
> terribly verbose, so we just have to hope we get it right :-)
I actually had a look into the default ActivePerl docs to find out 
whether there is a better way for generating xml, but as there is no 
XML-generator package in the default distribution I decided not to 
introduce a new dependency.

Thanks for your feedback.

Regards,

Brar