Skip to content

Conversation

@mmathieum
Copy link
Member

No description provided.

@mmathieum mmathieum self-assigned this Nov 25, 2025
mmathieum added a commit to mtransitapps/parser that referenced this pull request Nov 25, 2025
mmathieum added a commit to mtransitapps/commons-android that referenced this pull request Nov 25, 2025
@mmathieum
Copy link
Member Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 introduces new feature flags and database definitions to support exporting trip_id. The changes are mostly correct, but I've identified a couple of areas for improvement. Specifically, the new T_TRIP_IDS table is missing a primary key definition, which is crucial for data integrity and foreign key relationships. I've also suggested a minor formatting improvement to adhere to common file conventions.

mmathieum and others added 2 commits November 25, 2025 15:49
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@mmathieum mmathieum marked this pull request as ready for review November 25, 2025 20:50
@mmathieum mmathieum requested a review from Copilot November 26, 2025 16:41
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces support for exporting trips.trip_id by adding a new trip IDs table and related feature flags. The changes enable mapping between integer trip IDs and string trip IDs, similar to the existing service IDs functionality.

Key Changes:

  • Added new trip_ids table with SQL creation, insertion, and drop statements
  • Introduced feature flags for trip ID integers and trip ID arrival exports
  • Renamed unescapeStringOrNull to unquotesUnescapeStringOrNull for clarity

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/main/java/org/mtransit/commons/GTFSCommons.kt Added trip_ids table schema and updated comments for service_ids and strings tables
src/main/java/org/mtransit/commons/FeatureFlags.kt Added feature flags for trip ID exports with WIP annotations
src/main/java/org/mtransit/commons/sql/SQLUtils.kt Renamed function from unescapeStringOrNull to unquotesUnescapeStringOrNull

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mmathieum
Copy link
Member Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 introduces new feature flags and database definitions to support exporting trips.trip_id. The changes are generally good, including resolving a TODO for T_SERVICE_IDS_SQL_CREATE and a small refactoring in SQLUtils.kt. I've left a couple of suggestions for improvement: one to remove a redundant boolean operation for clarity, and another to address code duplication of constants to improve maintainability.

@mmathieum mmathieum requested a review from Copilot November 28, 2025 14:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mmathieum mmathieum merged commit ee67a2d into master Nov 28, 2025
4 checks passed
@mmathieum mmathieum deleted the mm/trip_ids branch November 28, 2025 19:08
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