Skip to content

ownsql: handle QByteArray without converting to QString#5097

Merged
ogoffart merged 1 commit into
masterfrom
ownsql
Aug 2, 2016
Merged

ownsql: handle QByteArray without converting to QString#5097
ogoffart merged 1 commit into
masterfrom
ownsql

Conversation

@ogoffart

@ogoffart ogoffart commented Aug 1, 2016

Copy link
Copy Markdown
Contributor

QByteArray is used for checksum

@mention-bot

Copy link
Copy Markdown

@ogoffart, thanks for your PR! By analyzing the annotation information on this pull request, we identified @guruz, @dragotin and @ckamm to be potential reviewers

@ogoffart

ogoffart commented Aug 1, 2016

Copy link
Copy Markdown
Contributor Author

( extracted from #5056 )

@dragotin

dragotin commented Aug 1, 2016

Copy link
Copy Markdown
Contributor

👍

Maybe we should remove the default case and error log so that we notice next time in case we introduce a new data type.

@guruz guruz added this to the 2.3.0 milestone Aug 1, 2016
@guruz

guruz commented Aug 1, 2016

Copy link
Copy Markdown
Contributor

Why not use sqlite3_bind_blob ?

@ogoffart

ogoffart commented Aug 2, 2016

Copy link
Copy Markdown
Contributor Author

Why not use sqlite3_bind_blob ?

I don't know? Why use it? Currently all the QByteArray only contains text.

@guruz

guruz commented Aug 2, 2016

Copy link
Copy Markdown
Contributor

OK 👍 if you take the risk that if we ever change that something bad happens.
Although the function has a size parameter so maybe it's OK.
(And we anyway always get text from WebDAV)

@ogoffart

ogoffart commented Aug 2, 2016

Copy link
Copy Markdown
Contributor Author

Any other than a string was not working before because we were converting to QString anyway.

@ogoffart ogoffart merged commit 73a6939 into master Aug 2, 2016
@ogoffart ogoffart deleted the ownsql branch August 2, 2016 09:39
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.

4 participants