Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert package to use abstraction #37

Merged
merged 5 commits into from
Mar 16, 2024

Conversation

guyluz11
Copy link
Contributor

@guyluz11 guyluz11 commented Feb 11, 2024

The code of this package should be based on the implementations of network_tools and override only the necessary parts that require flutter platform-specific override.

In order to achieve that I have changed the package to use inheritance of services, this will make development and code easier to maintain.

Another benefit is that users call the method exactly the same way as network tools

Old
network_tools: HostScanner.x();
network_tools_flutter HostScannerFlutter.x();

New
network_tools: HostScannerService.instance.x();
network_tools_flutter HostScannerService.instance.x();

To achieve that user needs to initialize the flutter package with this method configureNetworkToolsFlutter at the main function

Change:
configureNetworkTools --> configureNetworkToolsFlutter

This pr is ready but is blocked but based on osociety/network_tools#183 pr to network_tools to work so it is currently blocked.

Fix: #38

This pr does not change the package logic.

@guyluz11 guyluz11 marked this pull request as draft February 11, 2024 20:27
@git-elliot git-elliot marked this pull request as ready for review February 22, 2024 15:52
@guyluz11
Copy link
Contributor Author

The test crushed on using git dependency

warning • Publishable packages can't have 'git' dependencies • pubspec.yaml:44:5 • invalid_dependency

Should I wait for network_tools to update and then update this pr?
Or should I advance with it as it is

Copy link

codecov bot commented Mar 16, 2024

Codecov Report

Attention: Patch coverage is 48.48485% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 26.74%. Comparing base (df365b1) to head (b9f203e).

Files Patch % Lines
...vices_impls/host_scanner_service_flutter_impl.dart 0.00% 12 Missing ⚠️
lib/src/configure_flutter.dart 61.53% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev      #37      +/-   ##
==========================================
+ Coverage   22.38%   26.74%   +4.35%     
==========================================
  Files           3        4       +1     
  Lines          67       86      +19     
==========================================
+ Hits           15       23       +8     
- Misses         52       63      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@git-elliot git-elliot merged commit f6bb3ca into osociety:dev Mar 16, 2024
5 checks passed
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.

Improve architecture
2 participants