Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces support for indexing GTFS service IDs by adding a new service IDs table and enabling optional integer-based service ID columns in the service dates table, controlled by a feature flag.
Key Changes:
- Added new
service_idstable to map service ID strings to integers - Introduced
F_EXPORT_SERVICE_ID_INTSfeature flag to toggle between string and integer service IDs - Reorganized GTFS exception type constants for better code organization
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/main/java/org/mtransit/commons/GTFSCommons.kt | Added service IDs table schema, modified service dates table to support integer service IDs via feature flag, and reorganized exception type constants |
| src/main/java/org/mtransit/commons/FeatureFlags.kt | Added F_EXPORT_SERVICE_ID_INTS feature flag (default: false) to control service ID format |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a feature to use integer-based service IDs instead of strings, controlled by a feature flag. This is a good optimization for performance and storage. The implementation involves adding a new service_ids table to map string IDs to integer IDs. My review identified a few potential data integrity and consistency issues in the new SQL table definitions. I've provided suggestions to add a UNIQUE constraint, a missing foreign key, and to align the INSERT statement with the use of an AUTOINCREMENT column.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces changes to support indexing of service IDs by mapping them to integers, controlled by a new feature flag F_EXPORT_SERVICE_ID_INTS. This involves adding a new service_ids table and modifying the service_dates table to use integer foreign keys. The changes are mostly correct, but I've found a couple of issues. There's a logical error in the SQL INSERT statement for the new service_ids table that conflicts with the use of an auto-incrementing primary key. I've also suggested a minor refactoring to improve code clarity in the SQL CREATE statement for the service_dates table. Overall, good changes that will likely improve performance.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.