openmobilityfoundation / mobility-data-specification

A data standard to enable right-of-way regulation and two-way communication between mobility companies and local governments.
https://www.openmobilityfoundation.org/about-mds/
Other
676 stars 232 forks source link

MDS 2.0 - trip/fare attributes inconsistencies #860

Closed lastalfriday closed 1 year ago

lastalfriday commented 1 year ago

1 - typo

I'm assuming this is a typo?

Underscore please :)

https://github.com/openmobilityfoundation/mobility-data-specification/blob/release-2.0.0/modes/delivery-robots.md#trip-attributes

image

2 - typo

Please name these the same

https://github.com/openmobilityfoundation/mobility-data-specification/blob/release-2.0.0/modes/passenger-services.md#trip-attributes

image

https://github.com/openmobilityfoundation/mobility-data-specification/blob/release-2.0.0/modes/delivery-robots.md#trip-attributes

image

3 - unclear doc

The wording here contradicts eachother

image

4 - unclear doc

https://github.com/openmobilityfoundation/mobility-data-specification/blob/release-2.0.0/modes/delivery-robots.md#trip-attributes

image

https://github.com/openmobilityfoundation/mobility-data-specification/blob/release-2.0.0/modes/passenger-services.md#trip-attributes

image

5 - timestamp

https://github.com/openmobilityfoundation/mobility-data-specification/blob/release-2.0.0/modes/passenger-services.md#trip-attributes

image

6 - milliseconds

Overkill. Seconds should be more than enough here?

image

7 - typo

Underscore please

https://github.com/openmobilityfoundation/mobility-data-specification/blob/release-2.0.0/modes/passenger-services.md#fare-attributes

image

8 - currency

Type "currency" makes no sense? Unless "5 EUR" is valid, but then the type can just be "string"..

Maybe define a "Currency" object type?

{
  amount: <integer>,
  currency: <string>
},
{
  amount: 5,
  currency: "EUR"
}

..or something..

https://github.com/openmobilityfoundation/mobility-data-specification/blob/release-2.0.0/modes/passenger-services.md#fare-attributes

image

schnuerle commented 1 year ago

For 1, 2, 5, and 7 you could do a PR.

For 3, the 'may have' refers to the fact that you may have some combination of the following fields.

For 4, in passenger services it's a string because I can be a variety of sources.

For 6, MDS defaults to milliseconds as our common unit. I don't think we use seconds elsewhere.

For 8, instead of currency it should probably be an integer and in cents.

thekaveman commented 1 year ago

I don't see what the issue is with 5?

For 6 I think those should just be renamed timestamp since that's what we use everywhere; and yes, it is milliseconds since Unix epoch.

schnuerle commented 1 year ago

I think 5 was just formatting, fixed here.

6 I think leaving as milliseconds makes sense based on the field name and intention, because it's not the timestamp of when this happened, it's the calculated difference in timestamps.

schnuerle commented 1 year ago

For 2 the name is now consistent here.

schnuerle commented 1 year ago

7 is fixed here with underscore - Kegan does that change it form OpenAPI?

And 1 is fixed here, which also may need an OpenAPI update.

thekaveman commented 1 year ago

These items were all addressed in OpenAPI in https://github.com/openmobilityfoundation/mds-openapi/pull/36

schnuerle commented 1 year ago

Closing this multi-item issue. If you have ideas from this list that weren't addressed, please open a new issue to discuss them. Thanks!