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.

Sign up for GitHub

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

Jump to bottom

support defining columns with foreignIdFor #932

Merged
merged 6 commits into from
Sep 15, 2021
Merged

support defining columns with foreignIdFor #932

merged 6 commits into from
Sep 15, 2021

Conversation

Josh-G
Copy link
Contributor

@Josh-G Josh-G commented Sep 10, 2021 •
edited
Loading

  • Added or updated tests
  • Documented user facing changes
  • Updated CHANGELOG.md

Changes
This PR adds support for using ->foreignIdFor(YourModel::class) in migrations.

Breaking changes
None

@szepeviktor
Copy link
Collaborator

Thank you for your hard work!

@szepeviktor
Copy link
Collaborator

Nice to see you working, Josh!

@Josh-G
Copy link
Contributor Author

Josh-G commented Sep 14, 2021

test failures are unrelated (#934)

canvural
canvural requested changes Sep 15, 2021
View reviewed changes
Copy link
Collaborator

@canvural canvural left a comment

Choose a reason for hiding this comment

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

Also we can add @property string $address_id to the User model in tests. Then we can change the test to use the correct type instead of mixed

src/Properties/SchemaAggregator.php Outdated Show resolved Hide resolved
@canvural canvural merged commit fc9f2e2 into larastan:master Sep 15, 2021
@canvural
Copy link
Collaborator

Thank you!

@Josh-G Josh-G deleted the support-foreign-id-for branch September 15, 2021 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@szepeviktor szepeviktor szepeviktor approved these changes

@canvural canvural Awaiting requested review from canvural

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Josh-G @szepeviktor @canvural