floor icon indicating copy to clipboard operation
floor copied to clipboard

split floor into floor_common, floor, floor_ffi to be able to use floor without flutter.

Open rivasdiaz opened this issue 3 years ago • 4 comments

closes #637

  • splits floor in floor_common, floor_ffi and floor.
  • floor_common and floor_ffi do not depend on flutter or sqflite (only sqflite_common and sqflite_common_ffi)
  • floor depends on flutter, sqflite and sqflite_common_ffi, re-exports all floor_common

rivasdiaz avatar Feb 07 '22 07:02 rivasdiaz

thanks for implementing this improvement! i'll align our ci setup to these changes and eventually, i'll report back with suggested improvements.

vitusortner avatar Feb 15 '22 07:02 vitusortner

Thanks for considering my MR!

I've been successfully using my branch in a small project in which I wanted to write a dart only CLI to use on desktop.

One small issue I see is the generated code depends on sqfliteDatabaseFactory which needs to be initialized differently if using floor (flutter) or floor_ffi (dart only). The implication is the code declaring the database has to import one or the other. As a result, the database class cannot be added to a common package imported by both a flutter app or a dart only cli app. This doesn't seem to be a big issue IMHO (an abstract base class gets most of the job done), but just something for you to be aware.

If you have any concerns, I'll be happy to update this MR. Or feel free to use my MR as you see fit in case you want to use it as a basis to do your own MR.

Thanks!

rivasdiaz avatar Feb 16 '22 16:02 rivasdiaz

@vitusortner Could you please . merge this pull request

alimcomp avatar Aug 17 '22 12:08 alimcomp

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.31%. Comparing base (4146037) to head (28d05d9). Report is 1 commits behind head on develop.

:exclamation: Current head 28d05d9 differs from pull request most recent head 435813a. Consider uploading reports for the commit 435813a to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #638      +/-   ##
===========================================
+ Coverage    91.51%   94.31%   +2.79%     
===========================================
  Files           11       10       -1     
  Lines          224      211      -13     
===========================================
- Hits           205      199       -6     
+ Misses          19       12       -7     
Flag Coverage Δ
floor 94.31% <ø> (+2.79%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 25 '24 15:03 codecov[bot]

I see this PR checks in the generated code now. We did not have this before and they were git ignored, but only under /floor. Now that all this lives under floor_common, the gitignore must be updated. @hendrikvdkaaden Can you remove the generated files from git and after that update the gitignore file?

stephanmantel avatar Apr 25 '24 10:04 stephanmantel