Upgrade Drift to 2.23.0; start using new features #1248
Conversation
Signed-off-by: Zixuan James Li <[email protected]>
Signed-off-by: Zixuan James Li <[email protected]>
Signed-off-by: Zixuan James Li <[email protected]>
Signed-off-by: Zixuan James Li <[email protected]>
Signed-off-by: Zixuan James Li <[email protected]>
… 2.23.0 We will start requiring the more recent features added in releases since 2.5.0. Signed-off-by: Zixuan James Li <[email protected]>
The tests are adapted from the test template generated from `dart run drift_dev make-migrations`. This saves us from writing the simple migration tests between schemas without data in the future. For the test that upgrade the schema with data, with the helper, while it ended up having more lines than the original, the test becomes more structured this way. Signed-off-by: Zixuan James Li <[email protected]>
An alternative to this is `dart run drift_dev make-migrations`, which is
essentially a wrapper for the `schema {dump,generate,steps}`
subcommands. `make-migrations` let us manage multiple database schemas
by configuring them with `build.yaml`, and it dictates the
which subdirectories the generated files will be created at.
Because `make-migrations` does not offer the same level of customizations
to designate exactly where the output files will be, opting out from it
for now.
We can revisit this if it starts to offer features that are not
available with the subcommands, or that we find the need for managing
multiple databases.
See also:
https://drift.simonbinder.eu/migrations/step_by_step/#manual-generation
Signed-off-by: Zixuan James Li <[email protected]>
We previously missed tables that are not known to the schema. This becomes an issue if a new table is added at a newer schema level. When we go back to an earlier schema, that new table remains in the database, so subsequent attempts to upgrade to the later schema level that adds the table will fail, because it already exists. Testing for this is blocked until zulip#1172. Signed-off-by: Zixuan James Li <[email protected]>
| // on iOS, -> "Library/Application Support" via https://developer.apple.com/documentation/foundation/nssearchpathdirectory/nsapplicationsupportdirectory | ||
| // on Linux, -> "${XDG_DATA_HOME:-~/.local/share}/com.zulip.flutter/" | ||
| // | ||
| // This is reasonable for iOS per Apple's recommendation: |
There was a problem hiding this comment.
How about "This is the best fit for iOS per Apple's recommendation", if that's true? That would feel more satisfying than just saying it's a reasonable option, because then you wonder if there are other reasonable options and what those look like.
Same for Android and Linux below, and I don't yet follow your reasoning about Android. (Linux is clear from the removed line "That Linux answer is definitely not a fit.")
| // $ tools/check --fix drift | ||
| // and generate database code with build_runner. |
There was a problem hiding this comment.
Is there a specific command we can put here, or perhaps refer the reader to some documentation, even if it's just dart run build_runner --help? (The dart run seems helpful to a contributor arriving here for the first time.)
There was a problem hiding this comment.
I didn't include the command for this as there are different ways to generate them, and some are more efficient the the others. We should point to the relevant piece of documentation ("Generated files" in README.md), or move this section to the README.
| drift: ^2.5.0 | ||
| drift: ^2.23.0 |
There was a problem hiding this comment.
Need help checking if podfiles need an update; deps: Upgrade drift to 2.23.0
Confirmed by running tools/upgrade pod at this commit, on a Mac, that no changes to ios/Podfile.lock or macos/Podfile.lock are needed.
This prepares for #97 (PR #1167).