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

Improve strict_loading documentation [ci skip] #49850

Conversation

ansonhoyt
Copy link
Contributor

  • Update ActiveRecord::StrictLoadingViolationError API docs to
    reference the guide to help beginners learn how to resolve the error.
  • Update Active Record Query Interface guide to cover configuration and
    the association option.

Motivation / Background

This Pull Request has been created to improve the docs and guides.

Detail

This Pull Request changes:

  1. ActiveRecord::StrictLoadingViolationError API docs to link to the guide.
  2. Active Record Query Interface guide to cover other facets of strict_loading.

Additional information

Explains the association option, highlighted in Chris Oliver's excellent Rails World talk Powerful Rails Features You Might Not Know.

Continues the nice additions in #49329

I was inspired by @zzak's comment on #49741

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@ansonhoyt
Copy link
Contributor Author

The API Documentation Guidelines' Links section doesn't cover when or how we should link to the guides from API docs.

I think it would help me if I got this error, but I couldn't find any existing links from API docs to Guides, so let me know if I did a bad thing 😬

activerecord/lib/active_record/errors.rb Outdated Show resolved Hide resolved
activerecord/lib/active_record/errors.rb Outdated Show resolved Hide resolved
activerecord/lib/active_record/errors.rb Outdated Show resolved Hide resolved
guides/source/active_record_querying.md Show resolved Hide resolved
@eileencodes
Copy link
Member

Thanks, can you squash your commits into one commit?

* Update ActiveRecord::StrictLoadingViolationError API docs to
  reference the guide to help beginners learn how to resolve the error.
* Update Active Record Query Interface guide to cover configuration and
  the association option.

Apply suggestions from code review

Co-authored-by: Eileen M. Uchitelle <eileencodes@users.noreply.github.com>
@ansonhoyt ansonhoyt force-pushed the document-active_record-strict_loading_violation_error branch from 909dd7a to 5101170 Compare October 30, 2023 18:31
@ansonhoyt
Copy link
Contributor Author

@eileencodes Done! Thanks for bearing with my Codespaces learning curve.

P.S. I enjoyed your talk The Magic of Rails. It piqued my curiosity to explore Active Storage code as I write a small API client.

@eileencodes eileencodes merged commit 9483a19 into rails:main Oct 30, 2023
@ansonhoyt ansonhoyt deleted the document-active_record-strict_loading_violation_error branch October 30, 2023 18:54
@zzak
Copy link
Member

zzak commented Oct 31, 2023

Thanks for working on this! Good additions. 👏

The API Documentation Guidelines' Links section doesn't cover when or how we should link to the guides from API docs.

I think it would help me if I got this error, but I couldn't find any existing links from API docs to Guides, so let me know if I did a bad thing 😬

There is a tiny blurb about it here, if you mean what I think you mean:
https://edgeguides.rubyonrails.org/ruby_on_rails_guides_guidelines.html#linking-to-the-api

In short, api. and guides. get turned into edge* during generation. So we don't have to type "edge" unless we really mean it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants