Skip to content

Conversation

leek
Copy link

@leek leek commented Nov 1, 2024

No description provided.

Copy link
Owner

@chrisreedio chrisreedio 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 everything but that one line looks good.

@leek
Copy link
Author

leek commented Jan 28, 2025

I think everything but that one line looks good.

Fixed 👍

@leek leek changed the base branch from 3.x to 4.x September 17, 2025 16:14
@leek leek requested a review from chrisreedio September 17, 2025 16:16
@leek
Copy link
Author

leek commented Sep 17, 2025

@chrisreedio This is a super small PR that is fully backwards compatible. Can we get this one merged?

Copy link
Owner

@chrisreedio chrisreedio left a comment

Choose a reason for hiding this comment

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

If we're allowing customization of the Connected Account model, should we just depend on that model to set the table name?

Might make the migration kind of "weird" but would prevent any issues of setting the table name to one thing in the config and the model having a different one?

Still thinking about this a bit.

@leek
Copy link
Author

leek commented Sep 17, 2025

If we're allowing customization of the Connected Account model, should we just depend on that model to set the table name?

Might make the migration kind of "weird" but would prevent any issues of setting the table name to one thing in the config and the model having a different one?

Still thinking about this a bit.

This PR allows you to override the model or just override the table name. This is how Spatie packages handle this (example).

I just want the table name to be plural to follow Laravel standards without having to use a fork.

/cc @chrisreedio

@chrisreedio
Copy link
Owner

If we're allowing customization of the Connected Account model, should we just depend on that model to set the table name?

Might make the migration kind of "weird" but would prevent any issues of setting the table name to one thing in the config and the model having a different one?

Still thinking about this a bit.

This PR allows you to override the model or just override the table name. This is how Spatie packages handle this.

I just want the table name to be plural to follow Laravel standards without having to use a fork.

/cc @chrisreedio

Before this PR the table name was already plural?

@leek
Copy link
Author

leek commented Sep 17, 2025

Before this PR the table name was already plural?

Oh weird. I could have sworn it used to create connected_account. My mistake.

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.

3 participants