search

tjarvstrand / flutter_timezone

A fork of https://github.com/pinkfish/flutter_native_timezone
Apache License 2.0
42 stars 29 forks source link

feat: add Windows support #31

Closed domyd closed 3 months ago

domyd commented 3 months ago

Hi!

This adds support for the Windows platform, and fixes #22.

This is implemented using the ICU library that ships with Windows. Since I'm only including icu.h and linking icu.lib here, Windows versions earlier than Windows 10 1903 aren't supported. If this is an issue, this could be extended back to 1703 with more Windows-version-specific #includes and linking (as described in the link above).

I've split this up into three commits for easier reviewing 🙂

tjarvstrand commented 3 months ago

This is great, thank you!

Unfortunately, given that I don't have any way to test this and I have zero experience working with windows, my best assessment is basically that I can't see anything in here that would be outright harmful :D I'm just going to have to take your word for it that this is working as intended.

Would you say that this is ready to be merged?

domyd commented 3 months ago

Yeah, this is ready, I just did a final sanity check. And thanks for the trust, I guess 😄

As for testing: best way I can think of is doing it in CI by setting the runner's time zone and region to different values and checking that the plugin returns the correct IANA identifier. It doesn't look like it's too much effort, so if you want I can add that.

tjarvstrand commented 3 months ago

As for testing: best way I can think of is doing it in CI by setting the runner's time zone and region to different values and checking that the plugin returns the correct IANA identifier. It doesn't look like it's too much effort, so if you want I can add that.

Thanks, that would be brilliant!

domyd commented 3 months ago

I added the tests 🙂

The run_integration_test_windows.ps1 script has a set of test parameters and loops through them, setting the Windows system's time zone and region and then running the local_timezone_test.dart integration test and verifying that the output is as expected. I've tried to keep it fairly general so that extending these integration tests to run on other platforms in the future is as easy as possible.

To keep the integration test simple I split the timezone label into its own Text widget, and since I was already at it I touched up the UI just a little bit, and added a refresh button (which I found useful for development). Hope you don't mind!

tjarvstrand commented 3 months ago

Amazing, thanks!

tjarvstrand commented 3 months ago

Published as 2.1.0

domyd commented 3 months ago

Awesome, thanks for reviewing this so quickly :)