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

refactor(network_info_plus)!: two-package federated architecture #1235

Merged
merged 7 commits into from
Oct 14, 2022

Conversation

nohli
Copy link
Member

@nohli nohli commented Oct 13, 2022

Description

Refactor network_info_plus from the federated architecture to a platform interface and a package with all platform implementations.

Related Issues

#1226

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I titled the PR using Conventional Commits.
  • I did not modify the CHANGELOG.md nor the pubspec.yaml files.
  • All existing and new tests are passing.
  • The analyzer (flutter analyze) does not report any problems on my PR.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate that with a ! in the title as explained in Conventional Commits).
  • No, this is not a breaking change.

@nohli nohli changed the title efactor(network_info_plus)!: two-package federated architecture refactor(network_info_plus)!: two-package federated architecture Oct 13, 2022
@nohli
Copy link
Member Author

nohli commented Oct 13, 2022

Only the windows build is broken.
Could you please take a look @miquelbeltran, you fixed the windows build in a few commits here 🙂

@miquelbeltran
Copy link
Member

Sure! Will do in the upcoming days

@miquelbeltran miquelbeltran added Hacktoberfest Issues taking part in Hacktoberfest hacktoberfest-accepted labels Oct 14, 2022
@miquelbeltran
Copy link
Member

Alright windows build is fixed, but actually I wasn't able to retrieve a single value using the example, so I am not even sure this plugin works -at all- on windows :)

@nohli
Copy link
Member Author

nohli commented Oct 14, 2022

Alright windows build is fixed, but actually I wasn't able to retrieve a single value using the example, so I am not even sure this plugin works -at all- on windows :)

Thanks a lot!!

Why the if (!Platform.isWindows) { in example? Does it throw?

Could you please try if it works on main? If not, this PR at least wouldn't worsen anything.

During testing all plugins in the simulators, I often found values to be null. Thought, it's maybe a simulator thing. Or for example sensors on web.

@nohli nohli marked this pull request as ready for review October 14, 2022 14:12
@miquelbeltran
Copy link
Member

Why the if (!Platform.isWindows) { in example? Does it throw?

Yeah, it crashes, so it is impossible to run the example. These methods are not available on Windows as they are not implemented.

I will check with main as well for the other methods, but I suspect they don't work either. Maybe because I am on an ethernet connection? 😂

@miquelbeltran
Copy link
Member

Same thing on main

@nohli nohli merged commit c1edf75 into main Oct 14, 2022
@nohli nohli deleted the unfederate-network-info-plus branch October 14, 2022 17:55
@nohli nohli removed the Hacktoberfest Issues taking part in Hacktoberfest label Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants