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

Timetable snapshot code cleanup #6124

Merged

Conversation

vpaturet
Copy link
Contributor

@vpaturet vpaturet commented Oct 8, 2024

Summary

Code clean-up and documentation/clarification in TimetableSnapshot:

  • Clarify the name and purpose of the realTimeAdded* and realTimeModified* indexes: realTimeAdded* indexes contains data related to added trips (extra journeys), while realTimeModified* indexes contains data related to modified trips (that is: pre-existing scheduled trips modified by a real-time update).
  • Align getter names with the project naming convention.
  • Minor code clean-up.

Issue

#6048

Unit tests

No

Documentation

No

@vpaturet vpaturet added Technical Debt Real-Time Update The issue/PR is related to RealTime updates Skip Changelog labels Oct 8, 2024
@vpaturet vpaturet self-assigned this Oct 8, 2024
Copy link

codecov bot commented Oct 8, 2024

Codecov Report

Attention: Patch coverage is 72.72727% with 12 lines in your changes missing coverage. Please review.

Project coverage is 69.85%. Comparing base (07c72ea) to head (6113d1d).
Report is 27 commits behind head on dev-2.x.

Files with missing lines Patch % Lines
...a/org/opentripplanner/model/TimetableSnapshot.java 77.77% 1 Missing and 5 partials ⚠️
...entripplanner/apis/gtfs/datafetchers/StopImpl.java 0.00% 2 Missing ⚠️
...planner/transit/service/DefaultTransitService.java 66.66% 2 Missing ⚠️
...tripplanner/updater/siri/SiriFuzzyTripMatcher.java 50.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #6124      +/-   ##
=============================================
- Coverage      69.85%   69.85%   -0.01%     
+ Complexity     17497    17492       -5     
=============================================
  Files           1976     1976              
  Lines          74710    74706       -4     
  Branches        7645     7645              
=============================================
- Hits           52191    52187       -4     
  Misses         19868    19868              
  Partials        2651     2651              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vpaturet vpaturet force-pushed the timetable_snapshot_code_cleanup branch from cb53d81 to f91acf8 Compare October 8, 2024 14:12
@vpaturet vpaturet marked this pull request as ready for review October 8, 2024 14:17
@vpaturet vpaturet requested a review from a team as a code owner October 8, 2024 14:17
@vpaturet vpaturet force-pushed the timetable_snapshot_code_cleanup branch 3 times, most recently from 9681194 to 466c4a9 Compare October 9, 2024 07:50
@vpaturet vpaturet force-pushed the timetable_snapshot_code_cleanup branch from 466c4a9 to b451cbb Compare October 9, 2024 07:57
* TODO RT_AB: clarify if this is an index or the original source of truth.
*/
private final Map<TripIdAndServiceDate, TripPattern> realtimeAddedTripPattern;
private final Map<TripIdAndServiceDate, TripPattern> realTimeModifiedTripPattern;
Copy link
Contributor

Choose a reason for hiding this comment

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

My suggestion is to call this realTimeNewTripPatternsForModifiedTrips. It is long and ugly but at least it is very specific and shouldn't cause any confusion. We can rename it to something simpler later after we've cleaned up the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@vpaturet vpaturet force-pushed the timetable_snapshot_code_cleanup branch from 70a7ba3 to 3659670 Compare October 9, 2024 13:52
@@ -604,6 +600,12 @@ private void swapTimetable(TripPattern pattern, Timetable original, Timetable up
dirty = true;
}

private void validateReadOnly() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would call this validateNotReadOnly() or validateWritable() or throwIfReadOnly()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@vpaturet vpaturet force-pushed the timetable_snapshot_code_cleanup branch from eee401f to 28fc2e4 Compare October 9, 2024 14:04
@vpaturet vpaturet merged commit 9e58db4 into opentripplanner:dev-2.x Oct 10, 2024
5 checks passed
@vpaturet vpaturet deleted the timetable_snapshot_code_cleanup branch October 10, 2024 11:54
@t2gran t2gran added this to the 2.7 (next release) milestone Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Real-Time Update The issue/PR is related to RealTime updates Skip Changelog Technical Debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants