Skip to content

Conversation

remonh87
Copy link
Contributor

@remonh87 remonh87 commented Aug 11, 2021

I changed the structure a bit based on best practices from Google / Baseflow and Firebase plugins (NO ADDITIONAL FUNCTIONALITY IS CHANGED).

The structure:

  • example containing the example app
  • packages
    • flutter_reactive_ble => package users implements. This is now a pure dart package with business logic
    • reactive_ble_mobile => plugin for iOS and android containing the method channel implementation + protobuf stuff.
    • reactive_ble_platform_interface => the platform interface people need to implement for other platforms

todo:

  • add Melos (monorepo package management system).
  • update readme
  • fix CI
  • squash commits

@remonh87 remonh87 force-pushed the federated-plugin-structure branch from 9432cef to bde778c Compare August 12, 2021 05:40
@remonh87 remonh87 force-pushed the federated-plugin-structure branch from bde778c to 1a2f4bc Compare August 12, 2021 05:42
@remonh87 remonh87 force-pushed the federated-plugin-structure branch from 1a2f4bc to d384b79 Compare August 12, 2021 05:44
@PieterAelse PieterAelse self-assigned this Aug 12, 2021
Copy link
Collaborator

@PieterAelse PieterAelse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly some nits, so looks good 👍

Also tested the example app and runs/connects without issues.

@remonh87 remonh87 force-pushed the federated-plugin-structure branch 3 times, most recently from 2ddf4d6 to 52eec7b Compare August 13, 2021 04:41
@remonh87 remonh87 force-pushed the federated-plugin-structure branch from 52eec7b to 9dd18cc Compare August 13, 2021 04:54
@remonh87
Copy link
Contributor Author

@PieterAelse thanks for the review. I fixed all the comments can you look to it again and give your final verdict? I updated all versions to 4.0.0 since I want everything to be consistent (reactive_ble platform_interface v4.0.0 matches with flutter_reactive_ble 4.0.0. The CI is red because I still have links to the local dependencies but I ran all the commands before and everything Is in check. When you approve this I will start publishing it and the CI will be green (kind of chicken egg thingy).

@PieterAelse
Copy link
Collaborator

@remonh87 Approved with one missed comment still open, but not a biggie: #338 (comment)

@remonh87 remonh87 force-pushed the federated-plugin-structure branch from 1271468 to 6786434 Compare August 17, 2021 04:32
@remonh87 remonh87 merged commit e9b16c4 into master Aug 17, 2021
@remonh87 remonh87 deleted the federated-plugin-structure branch August 17, 2021 04:47
@juliansteenbakker
Copy link

Looks good! Just one question, right now all platform packages a visible on pub.dev. It would be better to unlist al plaform specific packages, and just list the front-facing flutter_reactive_ble package. Otherwise people will try to implement platform specific packages. You can change this on the pub.dev package admin panel.

@remonh87
Copy link
Contributor Author

@juliansteenbakker thanks for the tip I fixed it directly wasn't aware this is possible :)

jalpedersen pushed a commit to plyobits/flutter_reactive_ble that referenced this pull request Sep 30, 2021
* move current code one folder deeper in order to prepare the federated plugin structure

* create platform interface package

* move model and plugincontroller to platform interface

* remove moved code from main library

* create platform interface and integrate it in the reactive ble package

* remove debuglogging

* move protobuf related stuff out of the interface package

* change codemagic pipeline to support federated plugin structure

* make platform interface ready for publishing

* add docs for platform interface

* restructure project to best practice

* rename package flutter_reactive_ble_platform_interface to reactive_ble_platform_interface

* move mobile specific code into new reactive ble mobile package

* setup CI and melos

* update docs and prepare release

* fix dependencies after publishing
@maBarabas maBarabas mentioned this pull request Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants