Jonathan Pryor controllò:

> > +            foreach (var memberAttribute in 
> > context.Parameters.MemberAttributes)
> > +                WriteUsingNamespace(writer, GetNamespace(memberAttribute));
> 
> Opinions will vary, but I don't find long-named variables which are in
> scope for 1 line to be all that useful.  'a' would work just as well as
> 'memberAttribute', and (even better) would likely be more resilient to
> change.
> 
> (You don't need to make this change, I'm just mini-ranting. ;-)

I agree, but I've left it as is or I'd have to change the other loops
near this for consistency.

> I think some form of convention for parameters which take type names as
> values would be useful.  I'd consider a 'type' suffix, but we now have
> --generate-type, so that doesn't really work.
> 
> More importantly, there is another "compatibility" requirement: .NET
> SQLMETAL.EXE.  Using the same parameter names as .NET gives two things:
> 
> 1. Makes it easier to use DbMetal (we use the same options!)
> 2. Makes Mono's sqlmetal easier for me (we use the same options!) ;-)
> 
> For DbLinq extensions, this isn't as important, but this does
> potentially (unfortunately) impact some of your more recent work.  For
> example, SQLMETAL has a /entitybase option which, as per recent
> discussion, is pointless (entities are partial classes, why would you
> ever need this?!), but we need to keep it anyway (and I'm glad to see
> that you did).
> 
> Returning to the original point ("some form of convention"), I think
> that options taking type names as values should start with --with, thus
> instead of --schema-loader, we should use --with-schema-loader.

Done.

> > -                { "databaseConnectionProvider=",
> > -                  "Specify a custom IDbConnection implementation {TYPE}.",
> > +                { "dbconnection=",
> > +                  "IDbConnection implementation {TYPE}.",
> 
> Similarly, this should probably be --with-connection or
> --with-dbconnection.

Done.

> >                    type => DatabaseConnectionProvider = type },
> > -                { "sqlDialectType=",
> > -                  "The IVendor implementation {TYPE}.",
> > +                { "vendor=",
> > +                  "IVendor implementation {TYPE}.",
> 
> 1. As per above, a --with prefix.
> 2. I hate the 'vendor' terminology, as it doesn't really make sense to
> me.  There is lots of "cross-pollination" in supported SQL, e.g. SQLite
> supports SQL Server-style [...] quoting, plus `...` and "..." (to quote
> DB column and table names, etc.), and accepting SQL Server's '@'
> parameter prefix.  I suspect you can write a fairly decent amount of SQL
> queries that's identical between SQLite and SQL Server.
> 
> For this reason, I don't like the idea of tying "flavor of SQL" to
> "vendor" or manufacturer.  I prefer the term "dialect" (and am
> considering doing a s/DbLinq.Vendors/DbLinq.Dialects/ in some future
> version if insanity takes me).
> 
> So I'd prefer --with-sql-dialect.

Done.

> >                  { "dbml=",
> > -                  "Output as dbml to {FILE}. Cannot be used with /map 
> > option.",
> > +                  "Output as dbml to {FILE}.",
> >                    file => Dbml = file },
> 
> Have you removed the /map restriction?

No, but we do not have a /map option... :)

> >                  { "language=",
> >                    "Language {NAME} for source code: C#, C#2 or VB "
> >                    +"(default: derived from extension on code file name).",
> >                    name => Language = name },
> > -                { "aliases|renamesFile=",
> > +                { "aliases=",
> 
> I wonder if this should instead be --map (or have --map as an alias) --
> SQLMETAL provides a /map option, and iirc renamesFile was originally
> used for /map, so reverting this to /map would likely be best.

It seems that -aliases is used to read the file with the mapping, while
the SQLMetal -map does the opposite, it writes an XML file..

> > +                { "entity-attribute=",
> > +                  "{ATTRIBUTE} for entity classes in the generated code, "
> > +                  +"can be specified multiple times.",
> > +                  attribute => EntityAttributes.Add(attribute) },
> 
> But to follow up...is there really a need for --entity-attribute?  You
> can just place the attributes on a partial class declaration, AND
> SQLMETAL doesn't provide this option (no compatibility requirement), so
> I don't actually see a need for this anymore.

Removed.

> Thanks for looking into this!

Sorry for the delay.

I've also done some small adjustments to comments and unused code.

Reworked gzipped patch attached.

-- 
Emanuele Aina
Studio Associato Di Nunzio e Di Gregorio
http://dndg.it/
Via Maria Vittoria, 2
10123 Torino - Italy

--

You received this message because you are subscribed to the Google Groups 
"DbLinq" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/dblinq?hl=en.


Attachment: 0001-Cleanup-of-the-DbMetal-command-line.patch.gz
Description: GNU Zip compressed data

Reply via email to