On Fri, 2009-06-12 at 10:19 +0200, Federico Di Gregorio wrote: > Currently it is impossible to use the DbLinqProvider and > DbLinqConnectionType connection parameters with PostgreSQL because > Npgsql does not like extra aguments. The attached patch fixes this.
Four comments. 1. It would likely be easier to integrate if you worked against upstream DbLinq, if at all possible. http://code.google.com/p/dblinq2007/source/checkout 2. The resulting behavior of Vendor.CreateDbConnection() is broken. If the connection string contains only DbLinqProvider but not DbLinqConnectionType, it'll use the DbLinqProvider value as the DbLinqConnectionType value. This isn't bad, necessarily, though it will result in a confusing error message (complaining about DbLinqConnectionType when it isn't present). The worse behavior is that if *both* DbLinqConnectionType and DbLinqProvider are specified, the regex will extract the *first* one specified. Given a connection string of: DbLinqProvider=PostgreSQL; DbLinqConnectionType=SomeType it will try to use PostgreSQL as the ConnectionType, NOT SomeType, which will be wrong. 3. My bigger problem is a conceptual one. Usually it is preferable for the code that is parsing for a given parameter to remove it (if necessary), so that the entire code base doesn't need to be known to understand a local method. Specifically, I'm leery about Vendor needing to know about -- and remove -- the DbLinqProvider parameter, as Vendor doesn't actually care about that parameter (DbLinqProvider is needed in order to get the Vendor in the first place). If we add additional DbLinq parameters in the future, it means that we'll need to modify Vendor again as well, further spreading the logic for handling the parameter throughout the code base. Consequently, I would far prefer that the DbLinqProvider removal code be within DataContext, as that's where the parameter is searched for and used. 4. You should add unit tests to check the removal logic to .../src/DbLinq/Test/DataContextTest.cs, which also contains other tests for DbLinqConnectionType behavior. - Jon _______________________________________________ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list