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

Small documentation updates #430

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

crimson-knight
Copy link
Contributor

What does this PR do?

Adds an example for how to use the request parameter on relationships, and a few minor wording changes that improve the readability of the docs.

@crimson-knight
Copy link
Contributor Author

The example I used for the request parameter overlaps a bit with how STI is supposed to be used. I can make a better example if you'd prefer (like adding a 3rd column value to the request block).

I know this example is sprinkled across a few places in the docs, but when I was looking specifically at this section it was missing. So after figuring it out, I thought it would be good to add :)

… when running `crystal spec` and added the `pool_size` configuration to the configuration block example
belongs_to :person, foreign: :guid, primary: guid

# In this example model, the `type` field is a string of "mailing_address" or "physical_address"
mapping(
Copy link
Owner

Choose a reason for hiding this comment

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

Do you prefer to have mapping at the bottom of the class? Is it because it leaves more space for other information at the top of the file? Did you tested it with MTI?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This example came from how I was able to get Jennifer working with a DB that was already configured to work this way in a Rails app. It didn't properly use STI, but kind of mocked the behavior.

As far as where the mapping macro goes, I like to organize my models so the relationship definitions are at the top of the file, the mapping macro goes below that to make seeing the fields easy, and below that, I define any model methods. Just a personal preference because it makes finding relationships quick and easy.

Copy link
Owner

Choose a reason for hiding this comment

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

Currently all examples follow the opposite notation - mapping first and relations next. Could you please change the order here so it is consistent with other pages?

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

Successfully merging this pull request may close these issues.

None yet

3 participants