[OPEN-ILS-DEV] DB schema documentation: Patch for
040.schema.asset.sql
Dan Scott
denials at gmail.com
Fri Aug 15 19:54:53 EDT 2008
Josh:
2008/8/14 Josh Stompro <stomproj at larl.org>:
<snip>
> I have attached a patch to 040.schema.asset.sql that includes some standard
> block comments for each table, and SQL COMMENT ON statements for each table
> and field. I would like some feedback on what people think of this
> documentation method. On one hand, it keeps the documentation close to the
> source which should facilitate it being kept up to date. On the other hand
> it adds quite a bit of bulk to the schema files. And let me know what you
> think of how I worded the comments, I didn't see any comments for me to
> base mine on. I will include the DCO later after feedback and if the patch
> is still wanted.
Wow - on the whole, this patch looks really valuable! I wouldn't worry
about bulking up the schema files at all. In terms of correctness, I
haven't gone thoroughly through each comment.
As for feedback on the comment style: I might quibble a little bit
about consistency, like with the use of the term "OPAC" vs. "opac"
(and suggest that you replace all with "catalog" instead); similar
with "ID"/"id" (either use "ID" or use the less ambiguous
"identifier").
I'm torn a little bit on commenting on the effect of unique indexes
and the like. Part of me wants to say "that should be painfully
obvious from the DDL" and worries about those pieces, in particular,
getting out of sync with reality, but part of me also likes the
verbosity.
In some cases you have comments on tables (like asset.call_number) in
/* */ comments; in other cases (like asset.call_number_note) you use
COMMENT ON. I would be inclined to use COMMENT ON everywhere, unless
there's some reason you're shying away from that.
This is very good stuff, Josh, and will be very valuable for helping
other developers jump in. I would certainly encourage you to keep up
the good work.
--
Dan Scott
Laurentian University
More information about the Open-ils-dev
mailing list