Summary

Add some api for handling SQLite3 migrations

Review Request #1906 — Created Oct. 7, 2022 and submitted Oct. 10, 2022, 5:41 a.m.

Information

Owner

grim

Repository

pidgin/pidgin

Branch

default

Bugs

PIDGIN-17695

Depends On

Reviewers

Groups

pidgin

People

Description

Also port the SqliteHistoryAdapter to the new api.

Testing Done

Ran the unit tests under valgrind and checked out code coverage which is really just missing error handling I can't test.

Commits

Summary	ID
Add some api for handling SQLite3 migrations Also port the SqliteHistoryAdapter to the new api.	7c39089da3afc2bc0090629e37ccb5eaf083eb70

Issues

Description	From	Last Updated
An	QuLogic	Oct. 10, 2022, 5:18 a.m.
I thought we didn't put (out) on errors because it did nothing.	QuLogic	Oct. 10, 2022, 5:27 a.m.
its	QuLogic	Oct. 10, 2022, 5:18 a.m.
There's a word or something missing at the beginning of this sentence. I also don't understand what you mean by …	QuLogic	Oct. 10, 2022, 5:26 a.m.
creates	QuLogic	Oct. 10, 2022, 5:36 a.m.
its	QuLogic	Oct. 10, 2022, 5:36 a.m.
s/is to be/to be/	QuLogic	Oct. 10, 2022, 5:23 a.m.
s/Which of course/This/	QuLogic	Oct. 10, 2022, 5:23 a.m.
Since the transaction is deferred, is there really no chance of an error in the COMMIT?	QuLogic	Oct. 10, 2022, 5:27 a.m.
I don't know what 'pretend write' means here.	QuLogic	Oct. 10, 2022, 5:26 a.m.
OK, I do know why you do i+1, but this comment is confusing because it doesn't have anything to do …	QuLogic	Oct. 10, 2022, 5:36 a.m.

libpurple/purplesqlite3.h (Diff revision 1)
The issue has been resolved. Show all issues
```
An
```

libpurple/purplesqlite3.h (Diff revision 1)

The issue has been resolved. Show all issues

I thought we didn't put (out) on errors because it did nothing.

grim Oct. 10, 2022, 4:18 a.m.
```
What do you mean by "it did nothing" ?
```

QuLogic Oct. 10, 2022, 5:17 a.m.

It didn't affect the docs (and possibly not even the gir) because errors are special-cased.

grim Oct. 10, 2022, 5:24 a.m.
```
Ah gotcha, I forgot about that!
```

libpurple/purplesqlite3.h (Diff revision 1)
The issue has been resolved. Show all issues
```
its
```

libpurple/purplesqlite3.h (Diff revision 1)

The issue has been resolved. Show all issues

There's a word or something missing at the beginning of this sentence.

I also don't understand what you mean by a "full" migration, as it seems like the second entry in the below example is a "partial" schema since it only contains 1->2 changes, but then it's still an incremental migration for each step, and not a full one, unless full has some specific DB meaning.

Also, since you begin a transaction around the migration, you may want to mention not needing that here.

libpurple/purplesqlite3.h (Diff revision 1)
The issue has been resolved. Show all issues
```
s/is to be/to be/
```
libpurple/purplesqlite3.h (Diff revision 1)
The issue has been resolved. Show all issues
```
s/Which of course/This/
```

libpurple/purplesqlite3.c (Diff revision 1)

The issue has been resolved. Show all issues

Since the transaction is deferred, is there really no chance of an error in the COMMIT?

libpurple/purplesqlite3.c (Diff revision 1)
The issue has been resolved. Show all issues
```
I don't know what 'pretend write' means here.
```

Change Summary:

address everything i didn't comment on here.

Commits:

	Summary	ID
	Add some api for handling SQLite3 migrations Also port the SqliteHistoryAdapter to the new api.	a4d5b256b4f5f9d258bf631ad1faa9603e9bf168
	Add some api for handling SQLite3 migrations Also port the SqliteHistoryAdapter to the new api.	387d60a6ed5d0048bfa047404426d8ff8476e1ac

Diff:

Revision 2 (+1786 -64)

Show changes

	libpurple/meson.build
	libpurple/purplesqlite3.h
	libpurple/purplesqlite3.c
	libpurple/purplesqlitehistoryadapter.c
	libpurple/resources/sqlitehistoryadapter/01-schema.sql
	libpurple/tests/meson.build
	libpurple/tests/sqlite3/initial.sql
	libpurple/tests/sqlite3/malformed.sql
	4 more

Change Summary:

grossly simplify running the actual migrations.

Commits:

	Summary	ID
	Add some api for handling SQLite3 migrations Also port the SqliteHistoryAdapter to the new api.	387d60a6ed5d0048bfa047404426d8ff8476e1ac
	Add some api for handling SQLite3 migrations Also port the SqliteHistoryAdapter to the new api.	58552a0a0181900fbe4359f3269b9b19b5cb4495

Diff:

Revision 3 (+1698 -64)

Show changes

	libpurple/meson.build
	libpurple/purplesqlite3.h
	libpurple/purplesqlite3.c
	libpurple/purplesqlitehistoryadapter.c
	libpurple/resources/sqlitehistoryadapter/01-schema.sql
	libpurple/tests/meson.build
	libpurple/tests/sqlite3/initial.sql
	libpurple/tests/sqlite3/malformed.sql
	4 more

Change Summary:

drop (out) annotations

Commits:

	Summary	ID
	Add some api for handling SQLite3 migrations Also port the SqliteHistoryAdapter to the new api.	58552a0a0181900fbe4359f3269b9b19b5cb4495
	Add some api for handling SQLite3 migrations Also port the SqliteHistoryAdapter to the new api.	fda6d2de04b8c5a559856298d5c939211a12c528

Diff:

Revision 4 (+1698 -64)

Show changes

	libpurple/meson.build
	libpurple/purplesqlite3.h
	libpurple/purplesqlite3.c
	libpurple/purplesqlitehistoryadapter.c
	libpurple/resources/sqlitehistoryadapter/01-schema.sql
	libpurple/tests/meson.build
	libpurple/tests/sqlite3/initial.sql
	libpurple/tests/sqlite3/malformed.sql
	4 more

libpurple/purplesqlite3.h (Diff revisions 1 - 2)
The issue has been resolved. Show all issues
```
creates
```
libpurple/purplesqlite3.h (Diff revisions 1 - 2)
The issue has been resolved. Show all issues
```
its
```

libpurple/purplesqlite3.c (Diff revisions 1 - 2)

The issue has been resolved. Show all issues

OK, I do know why you do i+1, but this comment is confusing because it doesn't have anything to do with short circuiting the for loop. The for loop checks migrations[i], and version doesn't even exist as part of the loop condition.

Change Summary:

fix some typos reword some things

Commits:

	Summary	ID
	Add some api for handling SQLite3 migrations Also port the SqliteHistoryAdapter to the new api.	fda6d2de04b8c5a559856298d5c939211a12c528
	Add some api for handling SQLite3 migrations Also port the SqliteHistoryAdapter to the new api.	7c39089da3afc2bc0090629e37ccb5eaf083eb70

Diff:

Revision 5 (+1700 -64)

Show changes

	libpurple/meson.build
	libpurple/purplesqlite3.h
	libpurple/purplesqlite3.c
	libpurple/purplesqlitehistoryadapter.c
	libpurple/resources/sqlitehistoryadapter/01-schema.sql
	libpurple/tests/meson.build
	libpurple/tests/sqlite3/initial.sql
	libpurple/tests/sqlite3/malformed.sql
	4 more

Ship it!

```
Ship It!
```

Status:: Completed