Closed boskokg closed 4 years ago
Hey @boskokg :) I start taking a look but there is a lot of changes and as jaguar package doesn't have any test it's a bit hard to see if it's all ok :/ I'll have to pull your changes and make some manual tests.
Would you be ok to start making tests ? We can then put in place GitHub Actions to run them on any future PR to be more safe for the future.
Hey Jaumard, Thanks for the action. I can write tests in two weeks since I have a product production preparation now. Is it ok to merge this now, and write tests later?
No problem for merging this PR without tests, but I'll finish my review and make some manual tests see how this works as I have no notion of what v4 deeply change. I don't want to merge without making a minimal set of test on my side as it's totally new code to me ^^
I'll take care of this this weekend I think, if it's all good I'll merge.
So I was able to review more deeply the PR :) But not yet to test as the example in sqflite doesn't work (probably never worked as there is no flutter android/iOS app) so for now I'm having hard time testing the code, I'll try to setup the example to make it work, in the same time I've made few comments that we need to deal with before being able to merge this into master
So for now I can't have it to compile at all, I'm getting a bunch off weird errors
Compiler message:
../lib/src/adapter.dart:18:3: Error: Type 'Logger' not found.
Logger logger;
^^^^^^
../lib/src/adapter.dart:23:10: Error: Type 'Connection' not found.
Future<Connection> open() async {
^^^^^^^^^^
../lib/src/adapter.dart:33:39: Error: Type 'Connection' not found.
Future<T> run<T>(Future<T> Function(Connection<sqf.Database> conn) task,
^^^^^^^^^^
../lib/src/adapter.dart:33:39: Error: Expected 0 type arguments.
Future<T> run<T>(Future<T> Function(Connection<sqf.Database> conn) task,
^
../lib/src/adapter.dart:34:8: Error: Type 'Connection' not found.
{Connection<sqf.Database> withConn}) async {
^^^^^^^^^^
../lib/src/adapter.dart:34:8: Error: Expected 0 type arguments.
{Connection<sqf.Database> withConn}) async {
^
../lib/src/connection.dart:7:23: Error: Type 'Connection' not found.
class SqfConn extends Connection<sqf.Database> {
^^^^^^^^^^
../lib/src/connection.dart:7:23: Error: Expected 0 type arguments.
class SqfConn extends Connection<sqf.Database> {
^
../lib/src/connection.dart:11:58: Error: Type 'Logger' not found.
static Future<SqfConn> open(String path, {int version, Logger logger}) async {
^^^^^^
../lib/src/connection.dart:16:3: Error: Type 'Logger' not found.
Logger logger;
^^^^^^
../lib/src/compose/compose.dart:23:30: Error: Type 'Composer' not found.
class SqfComposer implements Composer {
^^^^^^^^
../lib/src/compose/create.dart:3:30: Error: Type 'DataType' not found.
String composeDataType(final DataType type) {
^^^^^^^^
../lib/src/compose/create.dart:27:30: Error: Type 'Property' not found.
String composeProperty(final Property col) {
^^^^^^^^
../lib/src/compose/expression.dart:47:23: Error: Type 'Literal' not found.
String composeLiteral(Literal literal) {
^^^^^^^
../lib/src/compose/expression.dart:73:20: Error: Type 'Func' not found.
String composeFunc(Func func) {
^^^^
../lib/src/compose/find.dart:52:31: Error: Type 'SelClause' not found.
String composeSelClause(final SelClause column) {
^^^^^^^^^
../lib/src/compose/row_source.dart:3:25: Error: Type 'RowSource' not found.
String composeRowSource(RowSource source) {
^^^^^^^^^
../lib/src/compose/row_source.dart:19:32: Error: Type 'AliasedRowSource' not found.
String composeAliasedRowSource(AliasedRowSource source) {
^^^^^^^^^^^^^^^^
../lib/src/compose/row_source.dart:31:19: Error: Type 'Row' not found.
String composeRow(Row row) {
^^^
../lib/src/compose/row_source.dart:38:22: Error: Type 'Values' not found.
String composeValues(Values values) {
^^^^^^
../lib/src/adapter.dart:12:7: Error: The non-abstract class 'SqfliteAdapter' is missing implementations for these members:
- Adapter.alter
- Adapter.connect
- Adapter.createDatabase
- Adapter.createTable
- Adapter.dropDb
- Adapter.dropTable
- Adapter.find
- Adapter.findOne
- Adapter.insert
- Adapter.insertMany
- Adapter.remove
- Adapter.update
- Adapter.updateMany
- Adapter.upsert
- Adapter.upsertMany
Try to either
- provide an implementation,
- inherit an implementation from a superclass or mixin,
- mark the class as abstract, or
- provide a 'noSuchMethod' implementation.
When you have time please add a working example under sqflite/example for me to test
More general note, first the v4 is still ongoing development and it's not finished at all. v4 in his current state doesn't offer more than v3, it's just refactoring stuff...
When I did Sqflite adapter, I didn't take that much example on what as been done for Postgres, I'm strongly disagree with all the short naming that complexify understanding, even more if new people want to contributes to the project.
I've created a legacyV3
branch based on the last stable v3 release and have fixed some problem related to foreign keys. I'll heavily maintain that version as I strongly tested it and it's on production in our app. I'll probably never migrate to v4 until it's totally done and tested because with v3, at start, we had lot of bugs so moving to a new untested version is suicide for a company.
So sure let's merge this PR, but for me it doesn't make it more stable that today as I'm not capable of running the example inside /sqflite/example
.
sqflite support for 4.x.x. minor fixes in other modules.