Skip to content

Use default max stop count for mode from routing defaults in transfer requests - #6494

Merged
optionsome merged 9 commits into
opentripplanner:dev-2.xfrom
HSLdevcom:maxStopCountForMode-default-fix
Mar 4, 2025
Merged

optionsome merged 9 commits into
opentripplanner:dev-2.xfrom
HSLdevcom:maxStopCountForMode-default-fix

Conversation

@optionsome

@optionsome optionsome commented Feb 26, 2025 •

Copy link
Copy Markdown
Member

Summary

Refactor max stop count limit so that the map is merged with any new definition of max stop counts. This also fixes an issue with default max stop count limit for mode being overridden by an empty map when using transfer cache requests.

This replaces #6488

Issue

No issue

Unit tests

Updated

Documentation

Not needed

Changelog

From title

This also fixes and issue with default values being overridden
@optionsome optionsome added !Bug Apply to issues describing a bug and PRs witch fixes it. +Bump Serialization Id Add this label if you want the serialization id automatically bumped after merging the PR labels Feb 26, 2025
@optionsome optionsome changed the title Fix an issue with max stop count limit with transfer requests Fix an issue with max stop count limit in transfer requests Feb 26, 2025
@codecov

codecov Bot commented Feb 26, 2025 •

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 94.44444% with 4 lines in your changes missing coverage. Please review.

Project coverage is 70.12%. Comparing base (5dcc881) to head (9ed9728).
Report is 102 commits behind head on dev-2.x.

Files with missing lines Patch % Lines
...ting/api/request/preference/MaxStopCountLimit.java 93.22% 0 Missing and 4 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #6494      +/-   ##
=============================================
+ Coverage      70.09%   70.12%   +0.03%     
+ Complexity     18293    18285       -8     
=============================================
  Files           2079     2078       -1     
  Lines          77781    77707      -74     
  Branches        7830     7836       +6     
=============================================
- Hits           54518    54492      -26     
+ Misses         20496    20436      -60     
- Partials        2767     2779      +12     

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

@optionsome
optionsome force-pushed the maxStopCountForMode-default-fix branch from 1875486 to 5889c90 Compare February 26, 2025 15:08
@optionsome
optionsome marked this pull request as ready for review February 26, 2025 16:33
@optionsome
optionsome requested a review from a team as a code owner February 26, 2025 16:33
@optionsome
optionsome requested review from VillePihlava and leonardehrenfried and removed request for VillePihlava February 26, 2025 19:35

@VillePihlava VillePihlava left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good. I could not find anything significant just a couple of nits

optionsome and others added 3 commits February 27, 2025 14:55
Co-authored-by: Ville Pihlava <ville.pihlava@gmail.com>

@leonardehrenfried leonardehrenfried left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I approve this but if you could give this a better PR title that would be nice.

@optionsome optionsome changed the title Fix an issue with max stop count limit in transfer requests Use default max stop count for mode from routing defaults in transfer requests Mar 4, 2025
@optionsome
optionsome merged commit 2c8cfdb into opentripplanner:dev-2.x Mar 4, 2025
@optionsome
optionsome deleted the maxStopCountForMode-default-fix branch March 4, 2025 12:53
t2gran pushed a commit that referenced this pull request Mar 4, 2025
t2gran pushed a commit that referenced this pull request Mar 4, 2025
@t2gran t2gran added this to the 2.7 milestone Mar 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

!Bug Apply to issues describing a bug and PRs witch fixes it. +Bump Serialization Id Add this label if you want the serialization id automatically bumped after merging the PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants