MobilityData / gbfs

Documentation for the General Bikeshare Feed Specification, a standardized data feed for shared mobility system availability. Maintained by MobilityData
https://gbfs.org
Other
794 stars 289 forks source link

Change field vehicle_type_id to vehicle_type_ids on geofencing_zones.features - version v3.0-RC #529

Closed pgiki closed 1 year ago

pgiki commented 1 year ago

I'm new to GBFS but thrilled about discovering this incredible shared mobility specification. As a huge fan of shared micro-mobility, I'm currently developing an open-source Django-based REST API using the GBFS schema. I'm finalizing the database schema and aiming to release the project soon.

I have a query regarding the "vehicle_type_id" within geofencing_zones.features (v3.0-RC). It seems to represent multiple vehicle type IDs, but elsewhere, it's referred to as "vehicle_type_ids". I'm curious why the singular form is used here; changing the key to vehicle_type_ids would likely sound more appropriate.

Please describe some potential solutions you have considered (even if they aren’t related to GBFS).

Change field vehicle_type_id to vehicle_type_ids on geofencing_zones.features

Is your potential solution a breaking change?


  {
    "geometry": { "#": "... Polygon A ..." },
    "properties": {
      "rules": [
        {
          "vehicle_type_id": ["bike", "scooter"], //change this field to "vehicle_type_ids"
          "ride_through_allowed": true
        }
      ]
    }
  },
  {
    "geometry": { "#": "... Polygon B ..." },
    "properties": {
      "rules": [
        {
          "vehicle_type_id": ["scooter"],
          "ride_through_allowed": false
        }
      ]
    }
  }
],
"global_rules": [
  {
    "ride_through_allowed": false
  }
]```
josee-sabourin commented 1 year ago

Hi @pgiki, welcome!

Great catch, this might have slipped through the cracks when we were updating geofencing_zones in v3.0, and should be plural to be consistent with the rest of the specification. Because this changes the name of a field, this would be considered a breaking change. Given that v3.0-RC has not been made official, we can roll this change into v3.0-RC2 along with #497 and #522.

To get this change into v3.0-RC2, it would need to pass a vote, to do so you can open a Pull Request. Governance states that the Pull Request must be open for a minimum of 7 days, at which point you, as the Advocate, can call a vote. Alternatively, MobilityData, as the maintainers, can take on the role of the Advocate and open the Pull Request and call the vote.

pgiki commented 1 year ago

Hi @josee-sabourin, thanks for the quick response and for seeing the suggestion useful. Alternative 2 where MobilityData opens the pull request is fine with me.

mplsmitch commented 1 year ago

This is something that's repeatedly caused confusion since vehicle types were introduced in v2.1. Having fields ( vehicle_type_id & vehicle_type_ids) with such similar names is a bad idea. I'd be supportive of renaming all the vehicle_type_ids to something else. For example we could change them to types_of_vehicles, the field definitions would remain the same. What do other people think, does this bother you as much as it does me? If folks agree I'll write the PR.

pgiki commented 1 year ago

For naming consistency I would say vehicle_type_ids is fine for this purpose. This is because other fields use the same singular-plural format. For example pricing_plan_id vs pricing_plan_ids on vehicle_types.json.

josee-sabourin commented 1 year ago

Closing this, please continue the discussion in #542