search

HaxeFoundation / record-macros

Macro-based ORM (object-relational mapping)
MIT License
49 stars 24 forks source link

merge with dbadmin library #40

Closed bablukid closed 5 years ago

bablukid commented 5 years ago

Hi, I propose to merge record-macros and dbadmin ( https://github.com/ncannasse/dbadmin ).

both libraries are tightly coupled and the code style is the same, so the fixes we'll make to record-macros can benefit to dbadmin ( and vice-versa )

Nicolas hasn't made any update since 2015, so I guess it will be easier for us if everything is at the same place, in a single repo.

technically I just copied the files from the original repo and isolated dbadmin in sys.db.admin. package ( it was in sys.db. )

-François

RealyUniqueName commented 5 years ago

Does anybody maintain this repo currently? @jonasmalacofilho ? If nobody is willing to step forward I can fix CI and merge this PR.

jonasmalacofilho commented 5 years ago

@RealyUniqueName

I am, even if I haven't been terribly active lately.

It's not immediately obvious to me what's what's causing that PHP error. Do you know?

On the subject of merging dbadmin: I'm not familiar with that codebase and thus not the best person to evaluate this ideas.

But it's a lot of code coming to a repo that already lacks interest/contributors, and the purpose of each projects is different. Record-macros is a dependency, dbadmin is (AFAIK) an application. I also see that it (usually) depends on hscript, which means that's it's likely not working on Haxe 4 right now.

P.S. I was going to ask Simon, but I just noticed I can enable recurrent rebuilds myself. This will help me find out about new errors sooner.

RealyUniqueName commented 5 years ago

CI issues is probably caused by mbstring php extension not enabled. I'll try to fix it. Regarding this PR I tend to agree with your arguments.

RealyUniqueName commented 5 years ago

CI has been fixed

jonasmalacofilho commented 5 years ago

Thanks @RealyUniqueName.

Regarding the adoption of dbadmin, I'll try using it for a bit before commenting anything else here.

bablukid commented 5 years ago

Hi, thanks for the feedback. dbadmin has mainly 2 purposes : 1/ act like a super simple phpMyAdmin : view , search , edit, delete records. It understands some specificities of record-macros like SEnum, SFlags, foreign keys (@:relation) 2/ make db migrations : sync record-macros objects definitions to MySQL : update field types, field names, add/remove indexes ...etc

Part 1 can be easily replaced by external tools like phpMyAdmin, adminer or MySQL Workbench ... I could stop using it and let that part sink in the deep sea of forgotten librairies :-)

But part 2 is super usefull, it generates the required SQL request to update my database shema , each time I modify my models. Like in this screenshot https://imgur.com/a/UsOWR9z

We could imagine a solution where we give up part 1, and we could move the code of part 2 to record-macros. Not the interface, just the feature. Then the developer is free to create the interface he wants for this : admin panel, CLI command ...

What do you think about this ?

RealyUniqueName commented 5 years ago

That sounds much better than merging dbadmin.

bablukid commented 5 years ago

Ok, I'll work on this and submit a new PR