Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing info for change column and table comment in Guides #44928

Merged
merged 1 commit into from Apr 22, 2022

Conversation

gustiando
Copy link
Contributor

@gustiando gustiando commented Apr 20, 2022

Summary

Some people (myself included) rely heavier on the guides for a quick reference more than the api docs. Thought I'd throw this one in there to see if it helps.

You can help improve the Rails guides or the API reference by making them more coherent, consistent, or readable, adding missing information, correcting factual errors, fixing typos, or bringing them up to date with the latest edge Rails.

This will address the "adding missing information" suggestion from the guide.

@rails-bot rails-bot bot added the docs label Apr 20, 2022
Copy link
Member

@jonathanhefner jonathanhefner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to add these. Though it might be worth modifying "the change method supports only these migration definitions" to sound less final, in case of future missing entries like these.

When you are happy with the changes, would you mind squashing the commits?

guides/source/active_record_migrations.md Outdated Show resolved Hide resolved
guides/source/active_record_migrations.md Show resolved Hide resolved
@gustiando
Copy link
Contributor Author

gustiando commented Apr 21, 2022

Thanks for the guidance @jonathanhefner . I'm happier with the changes now.

I was considering linking to the api docs or something. But am curious what you think. I tried to avoid blowing up the scope of this change too much. Happy to accommodate if you have better suggestions though :)

@jonathanhefner
Copy link
Member

I was considering linking to the api docs or something. But am curious what you think. I tried to avoid blowing up the scope of this change too much.

Do you mean adding another link, in addition to the individual method links? I think it would be nice to do so, but there doesn't seem to be a definitive place to link to. Linking to SchemaStatements for its list of methods doesn't seem quite right because there are additional methods listed there. Adding some documentation to the SchemaStatements module itself might improve that, but, I agree, that would be out of scope for this PR.

This will ensure the guide is aligned with the doc on some things that I believe it's useful. Some folks do go for the guides more of than the api for a quick reference.

Order alphabetically

Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>

Add change colum and table comment links for reference in guide

Update  working so we know this list may keep changing
@gustiando
Copy link
Contributor Author

gustiando commented Apr 21, 2022

Do you mean adding another link, in addition to the individual method links?

@jonathanhefner yes, that's what I had in mind.

@jonathanhefner jonathanhefner merged commit bc9fa03 into rails:main Apr 22, 2022
@jonathanhefner
Copy link
Member

jonathanhefner commented Apr 22, 2022

Thank you, @gumatias! 🎉

Backported to 7-0-stable in 05e1f70.

jonathanhefner added a commit that referenced this pull request Apr 29, 2022
Add missing info for `change` column and table comment in Guides [ci-skip]

(cherry picked from commit bc9fa03)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants