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

Add a matcher API for filters in the transit service used for datedServiceJourneyQuery #5713

Merged
merged 6 commits into from
Sep 23, 2024

Conversation

eibakke
Copy link
Contributor

@eibakke eibakke commented Feb 29, 2024

Also make use of it in the DatedServiceJourneyQuery

This is the first simple implementation of a filter using the unified matcher API.

Issue

#5630

Unit tests

Added unit tests for each matcher and the new TripOnServiceDateMatcherFactory. Also ensured that the API in local runs behaves as expected.

Documentation

Added JavaDoc.

Copy link

codecov bot commented Feb 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.76%. Comparing base (9b4f86a) to head (f361f01).
Report is 1356 commits behind head on dev-2.x.

Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #5713      +/-   ##
=============================================
+ Coverage      67.81%   69.76%   +1.94%     
- Complexity     16574    17366     +792     
=============================================
  Files           1914     1970      +56     
  Lines          72389    74378    +1989     
  Branches        7442     7604     +162     
=============================================
+ Hits           49090    51887    +2797     
+ Misses         20780    19846     -934     
- Partials        2519     2645     +126     

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

Copy link
Member

@t2gran t2gran left a comment

Choose a reason for hiding this comment

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

Initial review. Focus on design - which loos good. Look forward to apply this to a more advanced query.

@eibakke eibakke requested a review from t2gran March 5, 2024 07:46
@eibakke eibakke marked this pull request as ready for review March 5, 2024 07:46
@eibakke eibakke requested a review from a team as a code owner March 5, 2024 07:46
@eibakke eibakke changed the title Add a matcher API for filters in the transit service Add a matcher API for filters in the transit service used for datedServiceJourneyQuery Mar 5, 2024
Copy link
Member

@leonardehrenfried leonardehrenfried left a comment

Choose a reason for hiding this comment

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

This looks good to me now. I hope I haven't requested anything that @t2gran will want to change back.

Copy link
Member

@t2gran t2gran left a comment

Choose a reason for hiding this comment

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

I have read through the main, code I have the test code left.

… DatedServiceJourneyQuery.

This is the first simple implementation of a filter using the unified matcher API.
…and in a logically consistent manner.

Also does List.copyOf instead of simple reassignment between TripOnServiceDateRequestBuilder and TripOnServiceDateRequest.
…estions from code review.

Also improves documentation generally.
@leonardehrenfried
Copy link
Member

I think a git rebase went wrong here.

@eibakke eibakke marked this pull request as ready for review September 6, 2024 07:16
@t2gran t2gran modified the milestones: 2.6, 2.7 (next release) Sep 18, 2024
@eibakke eibakke merged commit 49db57e into opentripplanner:dev-2.x Sep 23, 2024
5 checks passed
@eibakke eibakke deleted the trip-on-day-filter branch September 23, 2024 12:56
t2gran pushed a commit that referenced this pull request Sep 23, 2024
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.

3 participants