This project is archived and is in readonly mode.
[PATCH] change_table cleanup
Reported by Matthew Boehlig | May 4th, 2008 @ 07:23 PM
This patch cleans up the changes that were introduced with #71. http://github.com/rails/rails/co...
It affects three areas:
- code
- documentation
- 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)
Comments and changes to this ticket
-
Pratik May 11th, 2008 @ 09:25 PM
- 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.
-
Matthew Boehlig May 11th, 2008 @ 09:35 PM
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.
-
Matthew Boehlig May 20th, 2008 @ 06:46 AM
My change_table doc cleanup in docrails got merged in commit 46f30f90 so this ticket can be marked as resolved.
-
Pratik May 20th, 2008 @ 08:57 AM
- State changed from incomplete to resolved
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>