search

OneBusAway / onebusaway-ios

OneBusAway for iOS, written in Swift.
Other
74 stars 31 forks source link

Fix: issue 629 #714

Open aaryankotharii opened 4 months ago

aaryankotharii commented 4 months ago
This PR migrates regions storage from UserDefaults to FileManager. Same file structure followed as mentioned in issue description.

fixes issue: #629

Code Changes

CLAassistant commented 4 months ago

CLA assistant check
All committers have signed the CLA.

aaronbrethorst commented 4 months ago

Thank you @aaryankotharii! I'll review and get back to you soon!

aaryankotharii commented 4 months ago

Hello @ualch9, I have made the following changes as requested above.

  1. replaced PropertyListEncoder / Decoder with JSONEncoder / Decoder.
  2. replaced enum FileDestination logic with simple static constants in RegionsService.
  3. renamed FileManagerProtocol to RegionsServiceFileManagerProtocol.
ualch9 commented 4 months ago

Thank you @aaryankotharii! Apologies for the delayed turnaround in reviewing, I'll get to this ASAP.

aaryankotharii commented 4 months ago

no worries @ualch9, but there are a few migration issues I'm not sure how to address. for example, how to approach fetching custom regions, of existing users, which are stored in userdefaults ?

ualch9 commented 4 months ago

no worries @ualch9, but there are a few migration issues I'm not sure how to address. for example, how to approach fetching custom regions, of existing users, which are stored in userdefaults ?

Excellent question... @aaronbrethorst, do you have any insight on if custom regions are used enough for us to support a migration path?

aaronbrethorst commented 4 months ago

I would not worry about custom regions at this time. Thanks for checking!

aaronbrethorst commented 3 months ago

@aaryankotharii - need anything from me in order to get this wrapped up?