On 09/28/2011 03:53 PM, Brar Piening wrote:
Brar Piening wrote:

The attached patch includes documentation changes and excludes my versions of pgbison.pl and pgflex.pl which have been replaced by Andrews' versions that are already commited.

Building current head today I noticed that the patch doesn't apply cleanly anymore.

Attached is a new version.

This patch looks OK in principle.

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? 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?

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. 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 don't have a VS2010 machine available to test it on unfortunately. I'll see what I can do about arranging one, at least temporarily. Meanwhile I'll test it on my VS2005 and VS2008 machines to make sure it doesn't break anything.



Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to