Rename bikeRental options to vehicleRental

[2martens]

Summary

Renamed bikeRental options to vehicleRental

Issue

closes #5088

[leonardehrenfried]

You need to run mvn test to re-generate the docs. Then commit the changes and rerun mvn test.

[2martens]

Have updated the documentation and removed the remaining API-breaking change.

[leonardehrenfried]

The tests are failing because you have not comitted the updated docs.

[leonardehrenfried]

I also think that we should be moving these "vehicleRentalFoo" and "vehicleRentalBar" properties into their own JSON object.

[2martens]

The tests are failing because you have not comitted the updated docs.

I did update them but the changes within router-config.json were missing in the committed documentation changes. Will fix that.

Do you have an example of such a JSON object I could use as template?

[leonardehrenfried]

Check out https://github.com/opentripplanner/OpenTripPlanner/blob/f3b737cf4f9a1047d7826792818ecc7ba9fce4ee/src/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java

[leonardehrenfried]

The tests are failing because you have not comitted the updated docs.

I did update them but the changes within router-config.json were missing in the committed documentation changes. Will fix that.

Do you have an example of such a JSON object I could use as template?

I hope that there isn't someone on the core team who is super against doing that. If you'd be disappointed if you have to undo that work, you can wait until Tuesday to establish a consensus.

[codecov]

Codecov Report

Patch coverage: 89.55% and project coverage change: -0.01 ⚠️

Comparison is base (acd912d) 64.79% compared to head (8fb64dd) 64.78%.

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #5089      +/-   ##
=============================================
- Coverage      64.79%   64.78%   -0.01%     
- Complexity     14160    14162       +2     
=============================================
  Files           1733     1734       +1     
  Lines          67487    67504      +17     
  Branches        7221     7223       +2     
=============================================
+ Hits           43729    43734       +5     
- Misses         21330    21344      +14     
+ Partials        2428     2426       -2     

Impacted Files	Coverage Δ
...pplanner/graph_builder/module/osm/OsmDatabase.java	78.57% <0.00%> (-0.77%)	⬇️
...i/request/preference/VehicleRentalPreferences.java	96.00% <ø> (ø)
...alone/config/routerequest/VehicleRentalConfig.java	98.27% <98.27%> (ø)
...g/updaters/sources/VehicleRentalSourceFactory.java	86.66% <100.00%> (ø)
...dalone/config/routerequest/RouteRequestConfig.java	97.77% <100.00%> (-0.27%)	⬇️

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[2martens]

I looked at the mentioned class but don't yet understand how a VehicleRentalConfig would have to be wired in order for the config to have an effect. After all, it would still have to be within the route request, correct?

[leonardehrenfried]

You'd have to add it to RouterConfig which is then combined with the API parameters and together they make the RouteRequest which is passed to the routing engine.

[leonardehrenfried]

This is a very simple schematic how it works.

The implementation is surprisingly complex because there are a few layers of preferences that all need to be combined into a single RouteRequest.

[2martens]

Thanks for the info. Will take a look at it but likely not before Tuesday next week.

[2martens]

I tried to extract the options into a separate object. Hope it works like that.

[leonardehrenfried]

This looks good but I have 2 smaller requests.

[2martens]

Have implemented the requested changes