Andrew Dunstan wrote:

Some minor nitpicks:

Do we really need to create all those and files? They are all always included anyway. Why not just stash all the packages in and
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 and files are indeed not much more than stubs that could eventually be extended in future (and probably never will) contains the whole code for generating the new file format which would significantly bloat up the code in 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

   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.



Sent via pgsql-hackers mailing list (
To make changes to your subscription:

Reply via email to