-
Notifications
You must be signed in to change notification settings - Fork 0
Vehicles locations #12
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
base: master
Are you sure you want to change the base?
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 significantly refactors the GTFS data model by disentangling the 'Trip' concept from 'Direction', which were previously treated as interchangeable aliases. This change establishes 'Trip' as a distinct entity with its own dedicated database schema, improving clarity and laying groundwork for more granular data management. Additionally, new feature flags have been introduced to enable future optimizations related to service date export and schedule sorting, enhancing the system's flexibility and potential for performance improvements. Highlights
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request refactors the GTFS data model by separating the Trip and Direction concepts, which were previously conflated. This is a positive change for the data model's clarity. The implementation involves removing deprecated code and introducing a new table structure. The changes are mostly solid, but I've identified a couple of areas for improvement related to code consistency and maintainability. My feedback includes a high-severity comment on inconsistent column ordering in an SQL statement and a medium-severity comment on an incomplete region tag.
| val T_TRIP_SQL_INSERT = SQLInsertBuilder.getNew(T_TRIP).apply { | ||
| appendColumn(T_TRIP_K_ROUTE_ID) | ||
| appendColumn(T_TRIP_K_DIRECTION_ID) | ||
| appendColumn(T_TRIP_K_SERVICE_ID) | ||
| appendColumn(T_TRIP_K_ID) | ||
| }.build() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency with other SQL_INSERT definitions in this file, and with the T_TRIP_SQL_CREATE statement, the primary key column (T_TRIP_K_ID) should be listed first. All other insert statements in this file list the primary key first, and it's good practice to keep the column order consistent between CREATE and INSERT statements for better maintainability. This inconsistency could lead to confusion and potential bugs.
| val T_TRIP_SQL_INSERT = SQLInsertBuilder.getNew(T_TRIP).apply { | |
| appendColumn(T_TRIP_K_ROUTE_ID) | |
| appendColumn(T_TRIP_K_DIRECTION_ID) | |
| appendColumn(T_TRIP_K_SERVICE_ID) | |
| appendColumn(T_TRIP_K_ID) | |
| }.build() | |
| val T_TRIP_SQL_INSERT = SQLInsertBuilder.getNew(T_TRIP).apply { | |
| appendColumn(T_TRIP_K_ID) | |
| appendColumn(T_TRIP_K_ROUTE_ID) | |
| appendColumn(T_TRIP_K_SERVICE_ID) | |
| appendColumn(T_TRIP_K_DIRECTION_ID) | |
| }.build() |
| @JvmStatic | ||
| val T_TRIP_SQL_DROP = SQLUtils.getSQLDropIfExistsQuery(T_TRIP) | ||
|
|
||
| // endregion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh oh!
There was an error while loading. Please reload this page.