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

Fix/gh12894 single quote sqlite string literals #13257

Conversation

Swiftb0y
Copy link
Member

Alternative to #13247 along with some manual follow up trying to replace these literals in our query-strings as well. Also based on 2.4 instead of main (as #13247 currently is). I'd appreciate if someone could double check if I've missed any string.

@Swiftb0y Swiftb0y force-pushed the fix/gh12894-single-quote-sqlite-string-literals branch 3 times, most recently from b953954 to 643a3fa Compare May 22, 2024 06:44
@saper
Copy link
Contributor

saper commented May 22, 2024

I've found only those two so far, but I haven't reviewed everything yet.

@daschuer
Copy link
Member

@Swiftb0y Can you confirm that #13247 is now the base of this PR?
In that case I would prefer to merge both and rebase this one onto the original. This way we get around of the formal author/reviewer swap in this PR. Is this OK?

@Swiftb0y
Copy link
Member Author

@Swiftb0y Can you confirm that #13247 is now the base of this PR?

Yes.

In that case I would prefer to merge both and rebase this one onto the original. This way we get around of the formal author/reviewer swap in this PR. Is this OK?

Yes.

…ted are not disabled by default on SQlite >=3.41
@Swiftb0y Swiftb0y force-pushed the fix/gh12894-single-quote-sqlite-string-literals branch from 643a3fa to ccc3e69 Compare May 25, 2024 10:37
@Swiftb0y
Copy link
Member Author

rebased. please review.

@JoergAtGithub
Copy link
Member

LGTM! Thank you!

@JoergAtGithub JoergAtGithub merged commit 3c70208 into mixxxdj:2.4 May 25, 2024
14 checks passed
@Swiftb0y Swiftb0y deleted the fix/gh12894-single-quote-sqlite-string-literals branch May 26, 2024 09:22
@Swiftb0y
Copy link
Member Author

Thank you

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

Successfully merging this pull request may close these issues.

4 participants