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

Fix version #457

Closed
wants to merge 4 commits into from
Closed

Fix version #457

wants to merge 4 commits into from

Conversation

skinkie
Copy link
Contributor

@skinkie skinkie commented Jun 27, 2023

This attempts to fix the issue #455.

Choices made:

  1. A Version element MUST exist in any file that uses versions.
  2. version="any" cannot be supported unless a Version with version="any" exists.
  3. This likely breaks a lot of examples too, so it is based on the other dataSourceRef branch.

@skinkie skinkie self-assigned this Jun 27, 2023
@skinkie skinkie added bug Technical mistake, inconsistency with the documentation, etc. enhancement non semantic enhacement: technical enhancement, etc. labels Jun 27, 2023
@skinkie
Copy link
Contributor Author

skinkie commented Jun 27, 2023

@ue71603 still too many examples that should be fixed first.

@Aurige
Copy link
Contributor

Aurige commented Sep 1, 2023

see my comments on #455
Also I don't agree with "version="any" cannot be supported unless a Version with version="any" exists." : "any" mean that you can refer to any version you got (having a version number or not).
But I would agree that an object defined with an id and a version="any" does not really mean anything (but it's harmless, and could be seen as an explicit mention to say that the provider does not manage versions)

@skinkie
Copy link
Contributor Author

skinkie commented Sep 1, 2023

It is not harmless. Especially because "any" has semantics. A default value specifically stating there is no version would work, since it hopefully wouldn't break constraint checking. And please don't suggest a "version number" NeTEx expectly doesn't have a version number, otherwise the version-attribute should not be a string.

@ue71603 ue71603 added this to the netex_1.2 milestone Dec 1, 2023
Pls check. I am not happy. "any" is not accepted in Datasource. and always having to put in Version is not fun either.
@ue71603
Copy link
Contributor

ue71603 commented Dec 12, 2023

@skinkie @Aurige check the last commit. I worked on two examples and this adds layers that I am not sure, if they are good. Also "any" is NOT interpreted as it should. I had to add Datasource with id and version="78" to make it work.

@Aurige
Copy link
Contributor

Aurige commented Dec 12, 2023

It still looks weird to me (the version of a version, and with Id=version as attribute) it can be a convention, but does not really mean something and can be misunderstood... I personally would really prefer things like <Version id="78"> for versions. Also defining a version with just an Id looks like a useless technical artefact to me: the version element was initially created to carry some semantics, not just to hold an Id.
This said, I agree that we need to clarify the semantic of "any"

@ue71603
Copy link
Contributor

ue71603 commented Apr 17, 2024

@skinkie will update this as we changed stuff already.

@ue71603
Copy link
Contributor

ue71603 commented Apr 17, 2024

@skinkie should I fix it before you start working? or will you do it anyhow?

@skinkie skinkie closed this Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Technical mistake, inconsistency with the documentation, etc. enhancement non semantic enhacement: technical enhancement, etc. group_decision_needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants