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: remove unused routes-at-stop agent #2116

Merged
merged 1 commit into from
Jul 31, 2024
Merged

Conversation

digitalcora
Copy link
Contributor

d52ead8, which introduced more comprehensive V3 API request caching, removed the only instance where put was ever called on this cache, so it was effectively unused.

After removing a few layers of indirection:

  • Stop.create_station_with_routes_map is renamed and relocated to Route.serving_stop. Unclear why the previous name suggested it would "create a map".

  • The similar-in-nature Route.fetch_routes_by_stop is renamed to Route.serving_stop_with_active so these names are aligned.

  • Logging is added to the retry logic of serving_stop so we can tell whether it's still being used. This seems like a workaround for a possible bug in the V3 API which may have since been fixed, and if it hasn't been, we'll know and can report it.

  • serving_stop_with_active is fixed to conform to its typespec (and the spec of LocationContext) by returning a map containing only route_id and active? fields, rather than a map version of the entire Route struct with the id field renamed. It appears nothing was accessing the extra fields.

d52ead8, which introduced more comprehensive V3 API request caching,
removed the only instance where `put` was ever called on this cache, so
it was effectively unused.

After removing a few layers of indirection:

* `Stop.create_station_with_routes_map` is renamed and relocated to
  `Route.serving_stop`. Unclear why the previous name suggested it would
  "create a map".

* The similar-in-nature `Route.fetch_routes_by_stop` is renamed to
  `Route.serving_stop_with_active` so these names are aligned.

* Logging is added to the retry logic of `serving_stop` so we can tell
  whether it's still being used. This seems like a workaround for a
  possible bug in the V3 API which may have since been fixed, and if it
  hasn't been, we'll know and can report it.

* `serving_stop_with_active` is fixed to conform to its typespec (and
  the spec of `LocationContext`) by returning a map containing _only_
  `route_id` and `active?` fields, rather than a map version of the
  entire `Route` struct with the `id` field renamed. It appears nothing
  was accessing the extra fields.
Copy link

Coverage of commit 921aeaf

Summary coverage rate:
  lines......: 45.2% (2953 of 6533 lines)
  functions..: 40.4% (1053 of 2609 functions)
  branches...: no data found

Files changed coverage rate:
                                                                          |Lines       |Functions  |Branches    
  Filename                                                                |Rate     Num|Rate    Num|Rate     Num
  ==============================================================================================================
  lib/screens/application.ex                                              |75.0%      4|50.0%     2|    -      0
  lib/screens/routes/route.ex                                             |60.0%     70|57.1%    21|    -      0
  lib/screens/stops/stop.ex                                               |55.1%     69|73.1%    26|    -      0
  lib/screens/v2/candidate_generator/dup/departures.ex                    |86.3%    146|75.0%    24|    -      0
  lib/screens/v2/candidate_generator/pre_fare.ex                          |44.4%     45|33.3%    18|    -      0
  lib/screens/v2/candidate_generator/widgets/elevator_closures.ex         | 0.0%     22| 0.0%     8|    -      0

Download coverage report

@digitalcora digitalcora marked this pull request as ready for review July 31, 2024 16:35
@digitalcora digitalcora requested a review from a team as a code owner July 31, 2024 16:35
Copy link
Contributor

@cmaddox5 cmaddox5 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 taking care of this!

@digitalcora digitalcora merged commit 297c302 into main Jul 31, 2024
11 checks passed
@digitalcora digitalcora deleted the cfg-remove-agent branch July 31, 2024 20:45
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