Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

[PATCH] change_table cleanup

#108

This patch cleans up the changes that were introduced with #71 [patch] change_table block. http://github.com/rails/rails/co...

It affects three areas:

  1. code
  2. documentation
  3. whitespace

1. Code

The documentation said reference/remove_reference could be used (in addition to their plural names) but these methods were missing. Added proper aliases and tests.

2. Documentation

Tried to make things more consistent.

Fixes:

  • Added previously mentioned reference/remove_reference to list of available transformations
  • Pluralized/Singularized Examples depending on number listed
  • Made example descriptions into small headers
  • Removed dangling See that was never completed
  • Changed See to SchemaStatements#add_timestamps as #timestamps doesn't exist
  • Removed single t.removes() example from doc as there is only a singular method makes sense

3. Whitespace

  • Changed sentence spacing from two spaces to one, since it gets collapsed anyways
  • Empty lines and dangling spaces removed in entire schema_definitions.rb file

I tried to make things more consistent and will appreciate feedback:

  • I added reference alias to be consistent with docs, but are all three aliases needed? (reference , references , belong_to) As a sentence, foo references/belongs_to bar sounds proper as "noun verb noun"
  • In contrast, t.removes zig should just be singular to be consistent with the other methods
  • Does the whitespace cleanup in the entire file dilute the focus of main fixes and should it be left to a separate patch?

Patch is attached or available in my fork at: git://github.com/thetamind/rails.git (change_table_doc_fix or master branch)

Reported by Matthew Boehlig · May 4th, 2008 @ 07:23 PM

State: resolved
Milestone: none
Assigned to: nobody
Importance: none

Activity

  1. Pratik
    Pratik
    • State changed from new to incomplete

    Nice work!

    I don't think a new alias 'reference' is needed. The fix should to be change the docs where it says 'reference'.

    Probably, you can make your doc fixes to docrails branch - http://github.com/lifo/docrails - Just send me ('lifo') a PM at github and I can add you to commit list.

    Thanks.

    May 11th, 2008 @ 09:25 PM

  2. Matthew Boehlig
    Matthew Boehlig

    Will do.

    I was trying to guess the author's original intention, but it makes sense to leave the API as is and I'll just cleanup the docs.

    May 11th, 2008 @ 09:35 PM

  3. Matthew Boehlig
  4. Matthew Boehlig
    Matthew Boehlig

    My change_table doc cleanup in docrails got merged in commit 46f30f90 so this ticket can be marked as resolved.

    May 20th, 2008 @ 06:46 AM

  5. Pratik
    Pratik
    • State changed from incomplete to resolved

    May 20th, 2008 @ 08:57 AM