Conversation
|
Bugs... |
skinkie
left a comment
There was a problem hiding this comment.
See CI, your changes require updating the examples.
Yes, I know. But my GitHub configuration only checked out the xsd, so I think I can't do it in this PR - could I edit it directly on the web? |
|
@Aurige you have approved a change that breaks backwards compatibility. In this case it replaces PropulsionType with PropulsionTypes. Why is this OK? |
|
@trurlurl consider updating this one as well <xsd:group name="RefuellingEquipmentGroup">
<xsd:annotation>
<xsd:documentation>Elements for REFUELLING EQUIPMENT.</xsd:documentation>
</xsd:annotation>
<xsd:sequence>
<xsd:element name="FuelType" type="FuelTypeEnumeration" minOccurs="0">
<xsd:annotation>
<xsd:documentation>The type of fuel used by a vehicle of the type.</xsd:documentation>
</xsd:annotation>
</xsd:element>
</xsd:sequence>
</xsd:group> |
That's a mistake on my side ! |
|
By the way, I already removed ParkingPointGroup & GaragePointGroup from the documentation |
|
Shall I revert PropulsionTypes and FuelTypes? I neglected the fact that they were in v1.2.2 already and considered them as provisional and "under construction". I still don't believe that anybody really would be using them, especially given the, in my view, strange collection of FuelTypeEnums (e.g., how to interpret "electricity" vs. "battery" vs. "electricContact", or the question we had about "tpg"?). |
|
@trurlurl the work is already done. Why would you cause more work? |
That's good news - that somehow didn't shine up at my side, thank you! |
|
@trurlurl the question for you is do you want to change RefuellingEquipmentGroup as well? |
@skinkie There isn't a list in there, so a priori no need to change. If the question is whether we'd need multiple fuel types here...? This of course depends how the fuel types are intended to be used - and my feeling is that this has possibly not been worked out yet (my previous comment about elecricity). At least it isn't cleary documented in the code and for me sort of a riddle. So I don't know what the correct answer would be. And therefore: no, I don't have the wish to change RefuellingEquipmentGroup. |
|
It is the only place there is no list at this point. And ever regular fuel station has more than one fuel nozzle per device, so I would say updating that would also make sense. |
No objections. |
|
@trurlurl task for you; please update the documentation :-) |
|
@Aurige agreed with the Refuelling Equipment upgrade too? |
|
That's fine for me (with the deprecation) ... but we cannot update the doc any-more (sent to CEN last week) |
|
@Aurige and how should we proceed? make two PR one with documented things and one with the rest? |
|
@Aurige the issue was assigned to you prior to you sending the document to CEN. |
I would just validate that PR and create an issue for 2.1 or 3.0 referring it as a reminder to update the doc (Ok that will be a small inconsistency, but that's unfortunately not the only one ;-) ) |
There was a problem hiding this comment.
A summary of my understanding of the changes so far, with a "newcomer eye":
- Replace PropulsionType by PropulsionTypes (N values, space-separated)
- Replace FuelType by FuelTypes (N values, space-separated)
- Fix typo on ProjectedFeatureRef (previously “ProjectedFeartureRef”)
- Replace Covered possible values: from true/false to indoors (default), outdoors, or covered
- Replace VehicleCategory by VehicleCategories
- Replace Weight type from LengthType to WeightType
- Remove ParkingPointGroup completely (unused)
- Remove GaragePointGroup completely (unused)
(EDIT: maybe worth copying into the PR description if I didn't miss anything)
This one looks good to me!
@skinkie since we're aiming for NeTEx v2.0 freeze, if everything is fine we could go ahead and merge, and create a separate ticket for the CEN documentation, wdyt?
|
No, as @Aurige mentioned this cannot continue in 2.0. Some of the changes can but not element name changes, as mentioned. |
|
A try to see what could be merged (RETRO-COMPATIBLE & UNUSED), vs not (BREAKING):
This would mean splitting the PR in 2. |
|
In my perspective: ProjectedFeartureRef should be fixed, Same for indoor/outdoor, And the Length/WeightType. Creating plurals not part of the documentation is a no-go for me. So iff part of documentation, we can do it, otherwise no way. If ParkingPointGroup and GaragePointGroup are not in use: we can clean that up. But as mentioned before, I rather have those things bluntly cut out in a 3.0 version. Stefan |
|
as said before that's fine for me (with the deprecation) ... but we cannot update the doc any-more (sent to CEN last week) |
|
We probably should:
|
GaragePointGroup and ParkingPointGroup are never used.
@nick-knowles I changed type of Covered in DeckSpace.