KeePassDX icon indicating copy to clipboard operation
KeePassDX copied to clipboard

Extract database code into a separate library

Open GianpaMX opened this issue 2 years ago • 11 comments

I love the app, I'm a happy user but also I'm an android developer and I would like to try out with kotlin multiplatform, I'm thinking in using the code to read the database and put it in its own library.

My initial idea is to create a new gradle module with the android library plug in applied. That module could publish an aar in a maven repo. The app module would still use the databse module as a gradle dependency but any other project could use it as a maven dependency.

I'm not sure if you are interested in a such extraction but it would have some advantages like keeping update the changes in the database code updated for everyone at the same time.

I already started to work on this and I can create a PR showing how I imagine it could work

GianpaMX avatar Jul 28 '22 15:07 GianpaMX

Actually, I had planned to refactor the code to separate the concepts in version 4.0 of the application, great minds think alike.

Yeah, a maven lib would be great, I wanted to do the refactoring as I went along because right now there are still a lot of interdependent classes but if you can move the work forward it's a good thing! :)

J-Jamet avatar Jul 28 '22 17:07 J-Jamet

Great I'll continue working on PR #1371 and I might tag you over there with specific questions

GianpaMX avatar Jul 29 '22 10:07 GianpaMX

Hey @J-Jamet, my PR is ready for review

GianpaMX avatar Aug 03 '22 18:08 GianpaMX

Thank you for your work. There are a lot of changes, and I also made some on my side in other branches of version 3.5.0 (feature/Hardware_Key). So I'll do a review when it will be released to prepare version 4.0.0 because there will surely be a big merge to do with the modifications that are not yet in the develop branch.

J-Jamet avatar Aug 04 '22 07:08 J-Jamet

oh! I see. What would be the best way to get my code merged?

Should I rebase it on top of develop and edit my PR to target develop instead of master?

GianpaMX avatar Aug 04 '22 10:08 GianpaMX

I have to finish the current development version first to reduce bugs, I would like to correct an attachments bug and other issues which will touch the code of the database package. Your code will be integrated in version 4.0.0 if the refactoring is OK.

So, if you are in a hurry, you can make the code compatible with the develop branch so that it is easier to rebase but as there are still changes to be made before the release of the next version, it's up to you if you want to deal with conflicts as they arise (with develop) or when the version is in production (with tag 3.5.0) later.

J-Jamet avatar Aug 05 '22 16:08 J-Jamet

Release 3.5.0 has 25 cards in to-do, any chance to add this to that release? I think by the time all those cards are done this PR will be too complicated to merge.

I will try to help with the development of 3.5.0 but in the meantime I have forked the repo and published the database library in jitpack

allprojects {
  repositories {
    // ...
    maven { url 'https://jitpack.io' }
  }
}
dependencies {
  implementation 'com.github.GianpaMX.KeePassDX:database:0.0.2'
}

GianpaMX avatar Aug 08 '22 11:08 GianpaMX

Release 3.5.0 has 25 cards in to-do, any chance to add this to that release?

No, that's what I've been telling you all along. I am obliged to separate the concepts and to do things gradually to avoid bugs, a code refactoring with the creation of a module requires a lot of testing, it touches the architecture of the application and therefore requires a major version of the application.

I think by the time all those cards are done this PR will be too complicated to merge.

It will be even more complicated to debug if there are blocking bugs added during the architecture change, believe me. I understand that you want a library quickly, but I can't afford to sacrifice the stability of the app. I'll try to make it fast to release version 3.5.0, there are some features that will touch the database package but we'll still get there.

You can already test with your database extracted in jitpack, if we keep the same call methods it should not be a problem for the clients.

What I would like to do is to do things in the right order for a good migration, and at the same time create the most stable library possible.

J-Jamet avatar Aug 08 '22 11:08 J-Jamet

Well, I'll restructure the tasks to prioritize the database module extraction.

I just merged a big feature (Yubikey) in the develop branch, so I'll have to check the conflicts with this one and with the attachments bug (#1346).

I will check each feature of the current version 3.5.0 project that are in the TODO that are not bug fixes and migrate them in the new version 4.1.0. That way they won't conflict.

The current version 4.0.0 which contains the big functionality of multiple database management (#209 a big feature that will also restructure the application) will become version 5.0.0 and the new version 4.0.0 will be dedicated to your pull request and database module extraction.

I think this is a good way to organize tasks.

J-Jamet avatar Aug 08 '22 12:08 J-Jamet

It will be even more complicated to debug if there are blocking bugs added during the architecture change, believe me

I believe you.

I will check each feature of the current version 3.5.0 project that are in the TODO that are not bug fixes and migrate them in the new version 4.1.0. That way they won't conflict.

The current version 4.0.0 which contains the big functionality of multiple database management (#209 a big feature that will also restructure the application) will become version 5.0.0 and the new version 4.0.0 will be dedicated to your pull request and database module extraction.

Thank you for accommodating 🙏

I can help fixing bugs/features I saw this one could be a good starting point. What do you think?

GianpaMX avatar Aug 08 '22 12:08 GianpaMX

I can help fixing bugs/features I saw https://github.com/Kunzisoft/KeePassDX/issues/1354 could be a good starting point. What do you think?

Yes, I added a comment to better understand the task. Thanks for your help

J-Jamet avatar Aug 08 '22 13:08 J-Jamet

Hey @J-Jamet, I saw the 3.5.0 beta release how is the plan going? In which commit should be good to start the database extraction task?

GianpaMX avatar Oct 07 '22 22:10 GianpaMX

Sorry @GianpaMX , I haven't been very available lately. Normally there won't be any structural changes to the application until version 4.0.0, so the last development commit or the last 3.5.0 tag... should be OK. The merges will not be complicated so derived.

J-Jamet avatar Oct 13 '22 15:10 J-Jamet

Hey @J-Jamet I just pushed new commits to the PR #1371 It's branched of develop

GianpaMX avatar Oct 27 '22 16:10 GianpaMX

Hey @J-Jamet I've been using the debug version for a while and it works as usual. Have you been able to take a look? Anu feedback that you could share?

GianpaMX avatar Jan 17 '23 18:01 GianpaMX

Hey @GianpaMX , Yes of course, excuse me I don't have much time for myself at the moment. I prioritize the implementation of the Yubikey in the first place to release the version without too many bugs and then I dedicate myself to your work. I know I'm taking time, but don't worry, your PR will be integrated.

J-Jamet avatar Jan 17 '23 18:01 J-Jamet

Awesome 😊

GianpaMX avatar Jan 27 '23 00:01 GianpaMX

The dependencies are strongly linked to Android, so for the moment, the division into modules allows us to separate the KeePass database code from its consumption, but not to generalize it to any system.

J-Jamet avatar Jun 12 '23 11:06 J-Jamet