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

Use top level sequel to qualify table and column name #802

Merged
merged 1 commit into from
Dec 20, 2023
Merged

Use top level sequel to qualify table and column name #802

merged 1 commit into from
Dec 20, 2023

Conversation

jnunemaker
Copy link
Collaborator

xref #800

@jnunemaker jnunemaker self-assigned this Dec 19, 2023
@jnunemaker jnunemaker mentioned this pull request Dec 19, 2023
Closed
bkeepers
bkeepers reviewed Dec 19, 2023
View reviewed changes
Copy link
Collaborator

@bkeepers bkeepers left a comment

Choose a reason for hiding this comment

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

Is there any way to replicate the error in tests?

@jnunemaker
Copy link
Collaborator Author

@bkeepers yeah I dont know, but my guess is that the symbol extension is more of a hack like I did long ago for plucky whereas this top level stuff is official and will be supported.

jmoglesby
jmoglesby approved these changes Dec 20, 2023
View reviewed changes
Copy link

@jmoglesby jmoglesby left a comment

Choose a reason for hiding this comment

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

This fix solved the issue we were having in #800

@jnunemaker jnunemaker merged commit 9deaac5 into main Dec 20, 2023
72 checks passed
@jnunemaker jnunemaker deleted the sequel-fix branch December 20, 2023 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bkeepers bkeepers bkeepers left review comments

@jmoglesby jmoglesby jmoglesby approved these changes

Assignees

@jnunemaker jnunemaker

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jnunemaker @bkeepers @jmoglesby