On Saturday, February 14, 2015 at 10:15:56 PM UTC-8, Matt Palmer wrote:
>
> Hi everyone, 
>
> I'm pretty chuffed to be able to announce the release of 
> sequel-pg-comment, 
> an extension for Sequel to support setting comments on all PgSQL object 
> types.  This allows you to comprehensively document your database schema, 
> and really make the entity-relationship diagrams produced by tools like 
> SchemaSpy *really* rock. 
>
> I'd appreciate any feedback people have, from the documentation (too much? 
> too nit-picky?), through the coding style, to the usability of the API. 
>  If 
> you use it and find it helpful, I'd love to hear about that, too. 
>
> Happy documenting! 
>
>
Here's the link for interested 
parties: https://github.com/mpalmer/sequel-pg-comment

Just some minor recommendations:

1) Don't create a Sequel::Extensions module, you should probably have this 
under Sequel::Postgres, since it is specific to PostgreSQL.

2) Don't manually try to split based on the double underscore.  Instead, 
use Dataset#schema_and_table, which will recognize Symbol, String, 
Sequel::SQL::QualifiedIdentifier, and Sequel::SQL::Identifier instances.

3) Have a different method or add second argument to comment_for to 
indicate column comments.  With the current API, you can't differentiate a 
request for a schema qualified table from a request for a column in a 
table. 

4) Dataset#comment_for without an argument should probably give you the 
comment for the table.  One easy hack for this is to have the default 
argument value be: return db.comment_for(first_source_table)

5) Consider using the Sequel DSL instead of strings for SQL where possible 
(e.g. in Database#comment_for).

6) Instead of overriding generator methods to recognize the :comment 
option, override the Database schema methods (e.g. 
Database#create_table_indexes_from_generator, which you are already 
overriding) to check for the :comment option.  That would probably simplify 
things quite a bit.

7) Raise Sequel::Error or an appropriate subclass, don't raise RuntimeError.

8) Don't use Database#execute (it's adapter-specific API).  Use #run if you 
don't need a return value, or #fetch.all if you need data returned.

I think this is a great idea.  Making it easy to get and set comments makes 
it more likely that people will actually document their databases, and that 
is a good thing.  I'm certainly guilty of not documenting my databases 
properly, simply because there wasn't a simple way to do so in the past.

Thanks,
Jeremy

-- 
You received this message because you are subscribed to the Google Groups 
"sequel-talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sequel-talk.
For more options, visit https://groups.google.com/d/optout.

Reply via email to