Re: Review of VS 2010 support patches

From: Brar Piening <brar(at)gmx(dot)de>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Craig Ringer <craig(at)postnewspapers(dot)com(dot)au>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Review of VS 2010 support patches
Date: 2011-11-29 21:32:01
Message-ID: 4ED54F51.6030402@gmx.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2011-11-29 21:32:42 Re: Patch - Debug builds without optimization
Previous Message Andres Freund 2011-11-29 20:55:52 Re: Avoiding repeated snapshot computation