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

Refactoring zones so that data is lazy loaded when needed #207

Merged
merged 3 commits into from
Sep 28, 2023

Conversation

samlown
Copy link
Collaborator

@samlown samlown commented Sep 28, 2023

  • This fixes the bug with the WASM binary which couldn't cope with a large amount of raw data.
  • Makes zone data loading more efficient.
  • Unfortunately implies that the Region JSON data no longer contains the zones directly, they have to be loaded manually.

@samlown samlown requested a review from cavalle September 28, 2023 11:41
Copy link
Contributor

@cavalle cavalle left a comment

Choose a reason for hiding this comment

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

Changes LGTM! If they work and fix the WASM issue, I wouldn't delay much releasing them.

Just a non-essential consideration: do we strictly need (to ensure WASM won't break) to also remove the zones from the build/regimes output files? If we don't, I think it will still be useful to have all the regime metadata in those files. They can be useful in the future to generate documentation, clients in other languages…

@samlown
Copy link
Collaborator Author

samlown commented Sep 28, 2023

@cavalle the regime zone data is something I was thinking about... the issue there is that both data sources would be included in the wasm with a significant file size increase (Colombia comes in at 256kb). In theory the raw data can still be extracted from each regions individual data files, but its not as good. I'm not sure what to do yet for this.

@samlown samlown merged commit ef12b6c into main Sep 28, 2023
2 checks passed
@samlown samlown deleted the zone-refactor branch May 16, 2024 19:39
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