[OPEN-ILS-DEV] DB schema documentation: Patch for 040.schema.asset.sql

[Mike Rylander]

Josh, I've finally gone through all the comments, and I've got it
ready to check in before 1.4.0.0rc2 with only a couple minor edits
(one dealing with a semantic change since you originally sent the
patch).  If you wouldn't mind resending the patch with the DCO
attached, I would love to commit this.  My hope is that it will help
shame me into going back through and providing more SQL-level comments
on more of the schema ... ;)

--miker

On Mon, Aug 18, 2008 at 5:15 PM, Josh Stompro <stomproj at larl.org> wrote:
> Dan Scott wrote:
>>
>> 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
>>
>
> Thanks.  I was kind of wondering if anyone would have the time to review
> this since it is kind of long.  I know all the developers are busy working
> on new and urgent stuff, and to spend 20+ minutes on something old is hard
> to justify.  I wonder if I did it in smaller chunks if it would work better.
>  A couple tables a week perhaps.
>>
>> 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 hate the term OPAC and I would be glad to change that, but the column
> names use the term OPAC so I stuck with that to be consistent with what is
> there.  That is a simple search and replace for the most part though, so no
> problem either way.
>
> I will certainly try to be more consistent with case, thanks for pointing
> that out.
>>
>> 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.
>>
>>
>
> I would be fine with leaving those out if there are strong feelings about
> it.  Having to write a comment for some of those made me look at them closer
> to make sure I understood them, or thought I understood them at least.
>  Indexes don't seem to be included in the pg-autodoc output anyway.
>>
>> 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.
>>
>>
>
> Each table has both a block comment and the COMMENT ON comment.  I was
> thinking that the block comments are easier to read when you are browsing
> the source, while the COMMENT ON comments are nicely integrated into the
> live database for usage from there.
>>
>> 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.
>>
>>
>
> Thank you very much for the feedback.
> Josh
>
>
> --
> Lake Agassiz Regional Library - Moorhead MN larl.org
> Josh Stompro               | Office 218.233.3757 EXT-139
> LARL Network Administrator | Cell 218.790.2110
>
>



-- 
Mike Rylander
 | VP, Research and Design
 | Equinox Software, Inc. / The Evergreen Experts
 | phone:  1-877-OPEN-ILS (673-6457)
 | email:  miker at esilibrary.com
 | web:  http://www.esilibrary.com