Conversation
Summary of ChangesHello @mmathieum, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new cleanup mechanism to remove unused trip IDs from the system. By integrating a Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to remove unused trip IDs, which helps in cleaning up data. The implementation adds a cleanupIds method in GSpec that iterates through trip IDs and removes those not associated with any route. A corresponding remove method is added to MTripIds. The logic is sound. I have one suggestion to improve the performance of the new cleanup logic by avoiding an unnecessary sorting operation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to remove unused GTFS trip IDs, which is a necessary cleanup step. The changes are logical, particularly the switch from HashSet to HashMap for mTrips to allow for efficient lookups.
I have a few suggestions to improve the implementation:
- Correct a minor typo in a log message.
- Refactor the trip merging logic to be more concise using
putIfAbsent. - Improve the memory efficiency of the unused trip ID removal process by avoiding the creation of a large intermediate list.
Overall, good work on addressing the need to prune unused IDs.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a feature to remove unused trip IDs, which is an important cleanup step. The implementation correctly changes the data structure for trips to a HashMap for efficient processing and adds the pruning logic. My review includes a suggestion to improve the performance of the new prune method in MTripIds.kt, which could be a bottleneck with large GTFS datasets. The rest of the changes look good and are consistent with the PR's objective.
- commons: CI: add frequently used gh command with inputs in comments - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
…parser': - commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - commons: Enable `mt-sync-code-data` workflow on PRs with automatic skip of data download and app release mtransitapps/commons#539 - commons: app_reinstall_downgrade_debug_latest.sh ignore - commons: Remove `setGitBranch` from `code_setup.sh` and `set_app_release_required.sh` mtransitapps/commons#542 - commons: Add script for reinstalling debug APK with version downgrade support mtransitapps/commons#526 - commons: Fix PR reference in commits do not support markdown links, just use URL mtransitapps/commons#541 - commons: Convert PR references to full URLs in submodule sync commits mtransitapps/commons#533 - commons: Fix archive.sh: remove old archives when current archive exists mtransitapps/commons#536 - commons: prepare_only.sh (archive_selection.sh & unzip_gtfs.sh) > remove old gtfs next data if URL not availabla anymore - commons: Build(deps): Bump the maps group with 3 updates mtransitapps/commons#530 - commons: Build(deps): Bump com.google.ads.mediation:vungle from 7.6.3.0 to 7.6.3.1 in the ads group mtransitapps/commons#529 - commons: Build(deps): Bump com.google.devtools.ksp from 2.3.4 to 2.3.5 in the kotlin-ksp-compose group mtransitapps/commons#528 - commons: Compat w/ Winnipeg transit provider - commons: Compat w/ Greater Sudbury MyBus API - commons: Compat w/ Regina Transit provider - commons: Compat w/ QuebecRTC provider - commons: Compat w/ STM.info providers - commons: Compat w/ network security config - commons: Compat w/ GRT provider - commons: Compat w/ CaEdmontonProvider - commons: Compat w/ Strategic Mapping provider - commons: AndroidManifest.xml gen > EXPORT_VEHICLE_LOCATION_PROVIDER FF (OFF) - commons: AndroidManifest.xml gen > compat w/ NextBus #VehicleLocationProvider - commons-android: BUG > map marker icons are greens (more visible w/ low DPIs) mtransitapps/commons-android#53 - commons-android: cleanup - commons-android: Change DEBUG constant to use environment variable mtransitapps/commons-android#52 - commons-java: cleanup - commons-java: Change DEBUG constant to use environment variable mtransitapps/commons-java#15 - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32 - parser: Reader > fix trip_ids file name - parser: Revert "MSchedule > clear head-sign instead of setting empty string" - parser: MSchedule > clear head-sign instead of setting empty string - parser: Current schedule > only look backward for single-day service if already present in current schedule... - parser: Change DEBUG constant to use environment variable mtransitapps/parser#29 - parser: Avoid data change > reading service dates > compat w/ flatten FF - parser: Export trip IDs [ints], service IDs, [schedule] strings FFs > delete old files if FF is ON->OFF - parser: Better clean mergedIds (+trip Ids)
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
- commons: CI: add frequently used gh command with inputs in comments - commons: Auto-trigger data sync when archive contains newer data than deployed mtransitapps/commons#537 - parser: fix compat with no last service calendar dates - parser: Remove unused trip IDs ints. mtransitapps/parser#31 - parser: Schedule > clear head-sign instead of setting empty string mtransitapps/parser#30 - parser: Data Changed > compat with service IDs regex mtransitapps/parser#32
Most GTFS trip IDs are not reused in new schedules so we need to remove unused ones every time.