Skip to content

Conversation

@optionsome
Copy link
Member

Summary

Remove unused MinimumTransferTimeIsDefinitive feature.

Issue

No issue

Unit tests

Updated tests

Documentation

Automatically updated

Changelog

Maybe

@optionsome optionsome added this to the 2.7 (next release) milestone Nov 26, 2024
@optionsome optionsome requested a review from a team as a code owner November 26, 2024 20:06
@optionsome
Copy link
Member Author

Something weird seems to happen in the tests for the pr in Github actions, seemingly feature that is enabled in OTPFeatureTest leaks into other tests, but it could some other random problem.

@leonardehrenfried
Copy link
Member

@2martens

@miklcct
Copy link
Contributor

miklcct commented Nov 27, 2024

I think this feature needs to be kept. Under some situations train company advertises a short connection even if the trains are sent into platforms of opposite ends of a station. It is needed if we set up a deployment with the aim to provide ticketable journeys.

@2martens
Copy link
Contributor

We use this feature and it is indeed quite important to make sure that the transfer times are as configured. In fact that is a requirement that we cannot ignore.

@optionsome
Copy link
Member Author

I think this feature needs to be kept. Under some situations train company advertises a short connection even if the trains are sent into platforms of opposite ends of a station. It is needed if we set up a deployment with the aim to provide ticketable journeys.

Problem with this parameter is that it's graph wide parameter. It affects all minimum transfer time definitions.

We use this feature and it is indeed quite important to make sure that the transfer times are as configured. In fact that is a requirement that we cannot ignore.

@leonardehrenfried originally created this feature flag because I got a request from a city that they would need something like this, but I never ended up using this since the city's problem was fixed by reducing transfer slack (I think). I wasn't aware that this was used elsewhere so I'm ok with closing this pr since this is used by you. I'll keep this open for a day so this can be discussed in a developer meeting tomorrow but I'll most likely close this pr after it.

@2martens
Copy link
Contributor

2martens commented Nov 27, 2024

I have another job-related meeting during the dev meeting time slot tomorrow, so I won't be able to join you, unfortunately.

@leonardehrenfried
Copy link
Member

You should be aware that making time for the dev meeting helps your cause when you want to influence OTP's direction.

@2martens
Copy link
Contributor

2martens commented Nov 27, 2024

I'm well aware of that. @bas-hbt will be there.

@optionsome
Copy link
Member Author

We will revisit this in one year.

@optionsome optionsome closed this Nov 28, 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.

4 participants