Skip to content

Fix for issue #57 and fixed a few lints. #58

Open
bsutton wants to merge 2 commits intotjarvstrand:mainfrom
bsutton:main
Open

Fix for issue #57 and fixed a few lints. #58
bsutton wants to merge 2 commits intotjarvstrand:mainfrom
bsutton:main

Conversation

@bsutton
Copy link

@bsutton bsutton commented Jan 29, 2026

No description provided.

@bsutton
Copy link
Author

bsutton commented Jan 29, 2026

added a new unit test to ensure the list of timezones was being pulled.

Copy link
Owner

@tjarvstrand tjarvstrand left a comment

Choose a reason for hiding this comment

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

Thanks for the PR and sorry for the delay. Had a baby about a month ago so things have been a little out of wack 🙂

Looks good overall! Just one minor comment.

if (values == null) {
return [_getLocalTimeZone()];
}
return values.toDart.map((value) => value.toDart).toList(growable: false);
Copy link
Owner

Choose a reason for hiding this comment

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

Please change this to return a standard, growable, list.

I'm all for immutable (and by extension fixed size) collections, but then it should be reflected in the function signature. Using a fixed size list here seems like premature optimization and since it's hidden behind the interface it would definitely bite someone at some point.

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.

2 participants