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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
