Skip to content

Add customEndAtSeconds and uuid fields to Period#38

Draft
tahini wants to merge 1 commit into
chairemobilite:v2cfrom
tahini:newLineFields
Draft

Add customEndAtSeconds and uuid fields to Period#38
tahini wants to merge 1 commit into
chairemobilite:v2cfrom
tahini:newLineFields

Conversation

@tahini
Copy link
Copy Markdown
Collaborator

@tahini tahini commented Apr 20, 2021

In transition, those fields are being added to allow to customize when a
period should end and the uuid field is useful to save the period in the
database.

In transition, those fields are being added to allow to customize when a
period should end and the uuid field is useful to save the period in the
database.
@tahini
Copy link
Copy Markdown
Collaborator Author

tahini commented Apr 20, 2021

DO NOT MERGE YET I'm putting the patch here, but I'd like to add some unit tests around it and wait to see if the uuid field is absolutely necessary (since it goes from database to the cache, but it does not need to be the other way around...)

@greenscientist
Copy link
Copy Markdown
Contributor

@tahini Is this still relevant ?

@tahini
Copy link
Copy Markdown
Collaborator Author

tahini commented Feb 22, 2022

Still relevant I guess, but didn't have time to look into it. Is the period even necessary? Or could we just keep trips hours?

@tahini tahini marked this pull request as draft February 22, 2022 18:32
@greenscientist
Copy link
Copy Markdown
Contributor

@tahini, we should just do this. Having the capnp file out of sync does not make sense. Either we remove them from the transition side or we merge this in TrRouting.

@tahini
Copy link
Copy Markdown
Collaborator Author

tahini commented Apr 30, 2024

@greenscientist There's still the question of what to do with those fields. Is there any use case to have the capnp file have more data that what is needed in trRouting? Or do we consider those files are "just" a cache and can/should contain the minimal information required to run? (especially if we plan to store them along with calculations for later re-use, it goes towards minimal information)

@greenscientist
Copy link
Copy Markdown
Contributor

From the TrRouting perspective, this file should be in sync.
The question to remove the field should be in the Transition side of things. So if we don't settle this idea shortly, I propose we sync the files and figure it out later

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.

2 participants