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

Handling of MultiPointType #443

Merged
merged 11 commits into from
Sep 11, 2024
Merged

Conversation

ue71603
Copy link
Contributor

@ue71603 ue71603 commented May 4, 2024

addresses: #442

@ue71603 ue71603 added the bug Something isn't working label May 4, 2024
@ue71603 ue71603 added this to the v2.0 milestone May 4, 2024
@ue71603 ue71603 changed the title Update OJP_Trips.xsd Handling of MultiPointType: urgent answer required May 4, 2024
OJP/OJP_Trips.xsd Outdated Show resolved Hide resolved
herlitze
herlitze previously approved these changes May 6, 2024
@ue71603
Copy link
Contributor Author

ue71603 commented Jun 29, 2024

@skinkie @trurlurl can ou agree?

@trurlurl
Copy link
Contributor

So we now have the parameter as de facto optional and we say that it is important and "should" be set - I think that is OK. But why not more consequently change the cardinality to 0:1 in order to clearly mark it as optional in the documentation table? That would require a slightly greater change in the documentation but it could not lead to confusion any more. (Do we have other places where an element of an optional part is mandatory?)

OJP/OJP_Trips.xsd Outdated Show resolved Hide resolved
Copy link
Contributor

@sgrossberndt sgrossberndt left a comment

Choose a reason for hiding this comment

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

I may be quite late to the party regarding the state of OJP 2.0, but the definitions in MultiPointType are not really clear to me. Renaming or removing somePoints is probably out of question, but we definitely should clarify the documentation. I hope this is still possible?

OJP/OJP_Trips.xsd Outdated Show resolved Hide resolved
OJP/OJP_Trips.xsd Show resolved Hide resolved
OJP/OJP_Trips.xsd Show resolved Hide resolved
@sgrossberndt sgrossberndt changed the title Handling of MultiPointType: urgent answer required Handling of MultiPointType Jul 1, 2024
sgrossberndt
sgrossberndt previously approved these changes Jul 1, 2024
OJP/OJP_Trips.xsd Outdated Show resolved Hide resolved
sgrossberndt
sgrossberndt previously approved these changes Aug 23, 2024
skinkie
skinkie previously approved these changes Aug 24, 2024
@sgrossberndt sgrossberndt force-pushed the bugfix/clarification_multipointtyp branch from 9bb9b1b to a135d31 Compare August 24, 2024 15:56
OJP/OJP_JourneySupport.xsd Outdated Show resolved Hide resolved
@sgrossberndt
Copy link
Contributor

Please resolve the conflicts so we can get this in.

@sgrossberndt sgrossberndt mentioned this pull request Aug 30, 2024
@ue71603 ue71603 dismissed stale reviews from sgrossberndt and skinkie via bda5cd0 September 2, 2024 10:54
OJP/OJP_JourneySupport.xsd Outdated Show resolved Hide resolved
@sgrossberndt sgrossberndt mentioned this pull request Sep 5, 2024
incorporated André's comment
@ue71603
Copy link
Contributor Author

ue71603 commented Sep 7, 2024

@sgrossberndt as soon as André is happy, we can merge this one and then also the release

@sgrossberndt
Copy link
Contributor

@sgrossberndt as soon as André is happy, we can merge this one and then also the release

@ue71603 What about the added minOccurs="0" as discussed in the issue 442

OJP/OJP_JourneySupport.xsd Outdated Show resolved Hide resolved
Copy link
Contributor

@sgrossberndt sgrossberndt left a comment

Choose a reason for hiding this comment

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

Only blocked regarding to minOccurs="0", otherwise I am fine with the change.

OJP/OJP_Trips.xsd Show resolved Hide resolved
@ue71603
Copy link
Contributor Author

ue71603 commented Sep 11, 2024

@sgrossberndt when ok for you, then we can merge.

@sgrossberndt sgrossberndt merged commit c214441 into develop Sep 11, 2024
2 checks passed
@sgrossberndt sgrossberndt deleted the bugfix/clarification_multipointtyp branch September 11, 2024 12:22
@sgrossberndt
Copy link
Contributor

I also just found out it had minOccurs="0" before until it was removed on 24.05.2023 at 14:43 in "More optimisation methods, reorganising filters and policy for Trip Request according to discussions (#353)".
Since that commit handled multiple things at once it is not clear to me whether this was done on purpose back then.

@sgrossberndt
Copy link
Contributor

Apparently it was done on purpose back then by @ue71603:
#353 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants