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.

Sign up for GitHub

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

Jump to bottom

Simplify element names of IDs #215

Merged
merged 13 commits into from
Jan 20, 2023
Merged

Simplify element names of IDs #215

merged 13 commits into from
Jan 20, 2023

Conversation

ue71603
Copy link
Contributor

@ue71603 ue71603 commented Jul 20, 2022 •
edited
Loading

Fixes #181 #179

@ue71603 ue71603 added enhancement New feature or request documentation labels Jul 20, 2022
@ue71603 ue71603 added this to the v2.0 milestone Jul 20, 2022
This was referenced Jul 20, 2022
Closed
Closed
AndreasAtSBB
AndreasAtSBB previously approved these changes Jul 25, 2022
View reviewed changes
@ue71603
Copy link
Contributor Author

ue71603 commented Jul 25, 2022

I will have to change all examples. And it will create major problems with other PR, that don't do it. So. I will do it asap when a lot of things have already be merged.

@skinkie
Copy link
Contributor

skinkie commented Aug 8, 2022

Is there a reason not to make this an attribute?

@ue71603
Copy link
Contributor Author

ue71603 commented Aug 8, 2022

Mostly history. I can change it to an attribute.

Aurige
Aurige previously approved these changes Aug 19, 2022
View reviewed changes
@sgrossberndt sgrossberndt changed the title Id's simpliefied Trip/TripLeg Simplify name of Id in Trip and TripLeg Aug 23, 2022
sgrossberndt
sgrossberndt previously approved these changes Aug 23, 2022
View reviewed changes
@ue71603
Copy link
Contributor Author

ue71603 commented Aug 27, 2022

Discussion
• #215
• #230

They both would align things more to what it should be. However, (a) we don’t do it everywhere and we should move the id to an attribute in that case (b) in breaks a lot of implementations (c) It does not really simplify anything in a heavy way. I agree: It is hygiene, but I am not sure, if we should do it.
Please advise. For now I will leave them alone

@ue71603 ue71603 mentioned this pull request Aug 27, 2022
Merged
@trurlurl
Copy link
Contributor

trurlurl commented Oct 1, 2022

Documentation updated.

@skinkie
Copy link
Contributor

skinkie commented Oct 18, 2022 •
edited
Loading

Also check references so <LineRef>NL:Line:1</LineRef> becomes <LineRef ref="NL:Line:1" />

@ue71603 ue71603 dismissed stale reviews from sgrossberndt, Aurige, and AndreasAtSBB via fccad1c October 26, 2022 13:25
AndreasAtSBB
AndreasAtSBB previously approved these changes Dec 20, 2022
View reviewed changes
Copy link

@AndreasAtSBB AndreasAtSBB left a comment

Choose a reason for hiding this comment

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

Changes about the Id look correct and examples are also fixed. (Just made an optical check, did'n run them.)

Aurige
Aurige previously approved these changes Jan 10, 2023
View reviewed changes
@ue71603 ue71603 dismissed stale reviews from Aurige and AndreasAtSBB via c95b518 January 20, 2023 16:25
@sgrossberndt sgrossberndt merged commit 7b2033f into changes_for_v1.1 Jan 20, 2023
@sgrossberndt sgrossberndt deleted the id-simpliefied branch January 20, 2023 16:27
@trurlurl
Copy link
Contributor

Question: As the type of those Ids were changed from xs:NMTOKEN to ObjectIdType - shouldn't we change the following as well?

TripSummaryStructure.TripId
TripFareProductReferenceStructure.FromTripIdRef, .ToTripIdRef, .FromTripLegIdRef, .ToTripLegIdRef
TripFareResultStructure.FromTripLegIdRef, .ToTripLegIdRef
(Perhaps more)

@skinkie
Copy link
Contributor

skinkie commented Jan 25, 2023

Sounds reasonable, also from a validating perspective.

@trurlurl trurlurl mentioned this pull request Jan 28, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sgrossberndt sgrossberndt sgrossberndt approved these changes

@skinkie skinkie skinkie approved these changes

@AndreasAtSBB AndreasAtSBB AndreasAtSBB left review comments

@herlitze herlitze herlitze left review comments

@Aurige Aurige Aurige left review comments

@dirkweisser dirkweisser Awaiting requested review from dirkweisser

Assignees

@ue71603 ue71603

Labels
breaking doc updated enhancement New feature or request
Projects
None yet
Milestone
v2.0
Development

Successfully merging this pull request may close these issues.
7 participants
@ue71603 @skinkie @trurlurl @Aurige @sgrossberndt @herlitze @AndreasAtSBB