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

manifest.json: include area, country_code #572

Merged
merged 7 commits into from
May 17, 2024

Conversation

futuretap
Copy link
Contributor

What problem does your proposal solve? Please begin with the relevant issue number. If there is no existing issue, please also describe alternative solutions you have considered.

As described and discussed in #520, this PR adds new area and country_code fields allowing consumers to select relevant feeds according to the user location. area is proposed as a GeoJSON MultiPolygon (instead of plain rectangles) since most stake holders seemed to prefer this format.

What is the proposal?

Add area and country_code to manifest.json.

Is this a breaking change?

  • Yes
  • No
  • Unsure

Which files are affected by this change?

manifest.json

@CLAassistant
Copy link

CLAassistant commented Dec 4, 2023

CLA assistant check
All committers have signed the CLA.

gbfs.md Outdated Show resolved Hide resolved
Co-authored-by: Fabien Richard-Allouard <[email protected]>
@josee-sabourin josee-sabourin mentioned this pull request Dec 4, 2023
3 tasks
gbfs.md Outdated
@@ -316,7 +316,9 @@ Field Name | REQUIRED | Type | Defines
\-&nbsp;`versions` | Yes | Array | Contains one object, as defined below, for each of the available versions of a feed. The array MUST be sorted by increasing MAJOR and MINOR version number.
&emsp;&emsp;\-`version` | Yes | String | The semantic version of the feed in the form `X.Y`.
&emsp;&emsp;\-`url` | Yes | URL | URL of the corresponding `gbfs.json` endpoint.

&emsp;&emsp;\-`area` | OPTIONAL | GeoJSON MultiPolygon | A GeoJSON MultiPolygon that describes the operating area. If `area` is supplied, then the record defines the area in which vehicles can be rented and returned.
&emsp;&emsp;\-`country_code` | OPTIONAL | String | The country code of the operating area. The field MUST not be specified if the operating area spans multiple countries.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not an expert on validation but it seems like not specifying a code would make things difficult. To validate an empty country_code you would need to verify that area does in fact span international borders. Maybe it would be better to require the use of a user-assigned 3166 alpa-2 code. We could choose one from the list and require it's use for any case where a system spanned international borders. Details on how that works here: https://en.wikipedia.org/wiki/ISO_3166-1_alpha-2#User-assigned_code_elements

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both fields are OPTIONAL so I don’t see an issue if one of the fields is left out. Imo, an empty country code should be avoided. Instead it should be left out.

We could also allow multi-country polygons together with a country_code. In this case, both conditions have to be met, so the operating area equals the multi polygon intersected with the country area. Would you prefer this? It’s really an edge case that should rarely ever occur but the language should of course be clear.

@mplsmitch
Copy link
Collaborator

Both fields are OPTIONAL so I don’t see an issue if one of the fields is left out. Imo, an empty country code should be avoided. Instead it should be left out.

I totally agree. What I'm suggesting is that if I'm publishing a manifest.json list that includes a number of systems, each with it's own country_code, it might make sense to have a reserved code, say XX. This would be used to indicate a system that spans multiple countries in stead of having to write an exception for that one list item where the country_code field should be left out. That way we would avoid the possibility of an empty country_code and the file would still be valid.

We could also allow multi-country polygons together with a country_code.

As area is currently defined, I would assume this would already be allowed. There's nothing to indicate that area should be limited to a single country. So then the question is, if you have a multi-country polygon, which country code would you use with it?

This is all very much an edge case so I'm fine with leaving it as it's currently written. If it ever comes up in the future we can easily fix it.

@futuretap
Copy link
Contributor Author

What I'm suggesting is that if I'm publishing a manifest.json list that includes a number of systems, each with it's own country_code, it might make sense to have a reserved code, say XX. This would be used to indicate a system that spans multiple countries in stead of having to write an exception for that one list item where the country_code field should be left out. That way we would avoid the possibility of an empty country_code and the file would still be valid.

We could allow JSON null for the country_code. I assume most parser implementations would resolve this to the same as leaving it out. Would be cleaner imo than specifying XX or multi or something similar.

@richfab
Copy link
Contributor

richfab commented Dec 5, 2023

A potential solution to avoid the conditional requirement could be to name the field country_codes and make it an array. So, for a system in a single country we would have "country_codes": ["DE"] and for a system that covers several countries "country_codes": ["DE", "FR"]. Looking forward to hearing your opinion.

@futuretap
Copy link
Contributor Author

I’m open about making it an array. The only gripe I have is when a system is worldwide or covers a very large number of countries (are there such systems?).
Not sure though if it’s actually a problem. In systems.csv we have a single country code for ages and it seems to work well.

@richfab
Copy link
Contributor

richfab commented Jan 5, 2024

I hereby call a vote on this proposal. Voting will be open for 10 full calendar days until 11:59PM UTC on Monday, January 15, 2024.
Please vote for or against the proposal, and include the organization for which you are voting in your comment.
Please note if you can commit to implementing the proposal.

@richfab richfab self-requested a review January 5, 2024 11:52
@tdelmas
Copy link
Contributor

tdelmas commented Jan 8, 2024

+1 From Fluctuo

@richfab
Copy link
Contributor

richfab commented Jan 12, 2024

Voting on this PR closes in 3 calendar days, at 11:59PM UTC on Monday, January 15, 2024. Please vote for or against the proposal, and include the organization for which you are voting in your comment.

@BredeD
Copy link

BredeD commented Jan 14, 2024

+1 from Entur

@richfab
Copy link
Contributor

richfab commented Jan 15, 2024

The vote remains open until quorum is met. Tagging recent contributors for visibility: @testower, @jkurzanski, @PierrickP, @cmonagle, @simonsolnes, @ezmckinn, @AntoineAugusti, @Cj-Malone, @ArashMansouri, @bdferris-v2, @HannesOlszewski.

Thank you for your involvement in the spec 🙏

gbfs.md Outdated Show resolved Hide resolved
gbfs.md Outdated Show resolved Hide resolved
@bdferris-v2
Copy link
Contributor

I appreciate the updates based on my feedback. Unfortunately, Google is a +0 on this proposal. Which is to say, I don't think we have an specific qualms with it (especially after the updates), but we don't anticipate using it either. So deferring votes to those who actually would.

@richfab
Copy link
Contributor

richfab commented Jan 25, 2024

Given that there are at least 1 vote each from a producer (ENTUR) and a consumer (Fluctuo), the third vote can come from the Advocate (@futuretap).

reference

@futuretap
Copy link
Contributor Author

Thanks, so +1 from Where To? / FutureTap.

@richfab
Copy link
Contributor

richfab commented Feb 14, 2024

With some delay on my side, this vote has now closed, and it passes!

Votes in favor:
Fluctuo (consumer)
Entur (producer)
Where To? (consumer)

There were no votes against.

This change will be part of v3.1-RC planned to be released in May, as per the version release cycle in the governance.

Thank you for your involvement in the GBFS spec 🙏

commit e0c48b2
Author: Fabien Richard-Allouard <[email protected]>
Date:   Mon Apr 29 11:13:47 2024 +0200

    Clarify vehicle_id persistence rule (MobilityData#632)

    This PR:
    - Makes the rule about vehicle_id persistence more explicit: rotation is between rentals (not every time the feed is loaded)
    - Updates a forgotten reference to free_bike_status.json (now vehicle_status.json)

    Thanks @testower for flagging!

commit 73d135d
Author: Fabien Richard-Allouard <[email protected]>
Date:   Tue Apr 16 15:22:18 2024 +0200

    Set the migration guide as the v3.0 Release Notes (MobilityData#628)

    This PR sets the migration guide article as the v3.0 Release Notes: https://mobilitydata.org/how-to-upgrade-to-gbfs-v3-0/

commit 457cb71
Author: Fabien Richard-Allouard <[email protected]>
Date:   Thu Apr 11 14:19:32 2024 +0200

    Update current version to v3.0 (MobilityData#625)

    * Withdraw PR MobilityData#546 Floating Interval in price

    * Withdraw PR MobilityData#457 Reservation Price

    * Remove -RC and -RC2 from the spec

    * Update current version number in spec

    * Fix earlier version URLs

    * Update current version in README to v3.0

    * Set version history URL to Release Notes

commit f4dc269
Author: Mitch Vars <[email protected]>
Date:   Thu Apr 11 01:10:58 2024 -0500

    Correction systems.csv (MobilityData#623)

    Fix missing csv comma

commit 940cd63
Author: Mitch Vars <[email protected]>
Date:   Tue Apr 9 11:11:26 2024 -0500

    Update systems.csv to remove dead feeds (MobilityData#622)

    * Update systems.csv

    Remove obsolete feeds

    * Update systems.csv

    * Add 4 feeds that now return status code 200

    ---------

    Co-authored-by: Fabien Richard-Allouard <[email protected]>

commit a77c5c2
Author: Fabien Richard-Allouard <[email protected]>
Date:   Tue Apr 9 15:40:21 2024 +0200

    Mark system_hours and system_calendar as removed (MobilityData#621)

    ## Problem
    The files system_hours.json and system_calendar.json were removed in MobilityData#328. However, the spec says these files were “deprecated”, which usually means permitted, but discouraged.

    ## Solution
    Replace "deprecated” with "removed" for the files system_hours.json and system_calendar.json in the spec.

commit 45e944a
Author: indigotachizawa <[email protected]>
Date:   Mon Apr 8 21:01:42 2024 +0900

    add new service 'kotobike' (MobilityData#620)

commit 2f5aaad
Author: Fabien Richard-Allouard <[email protected]>
Date:   Wed Mar 20 17:07:56 2024 +0100

    Authentication information for Voi Brussels (MobilityData#613)

    This PR adds authentication information for Voi Brussels.

    > Access and usage of Voi’s GBFS data is subject to the following licensing terms: https://www.voi.com/voi-licencing-agreement.

    More info on the Belgium NAP: https://www.transportdata.be/dataset/voi

commit a72d597
Author: Mitch Vars <[email protected]>
Date:   Wed Mar 13 12:03:36 2024 -0500

    Update Nextbike URLs (MobilityData#610)

    * Update Nextbike URLs

    * Update 4 additional Nextbike URLs

    ---------

    Co-authored-by: Fabien Richard-Allouard <[email protected]>

commit 4b4d3f7
Author: Fabien Richard-Allouard <[email protected]>
Date:   Wed Mar 13 14:31:38 2024 +0100

    Fix FAQ URL in issue template (MobilityData#609)

    This PR fixes the FAQ URL in the issue template.

    Before | After
    -- | --
    https://gbfs.mobilitydata.org/faq | https://gbfs.org/learn/faq/

commit b286e95
Author: Fabien Richard-Allouard <[email protected]>
Date:   Fri Mar 1 17:06:20 2024 +0100

    Update roadmap URL in README (MobilityData#605)

    This PR updates the roadmap URL in the README:

    - Before: Github wiki
    - After: https://portal.productboard.com/26qpteg4wct9px3jts94uqv8/tabs/99-planned

commit bea68ce
Author: Fabien Richard-Allouard <[email protected]>
Date:   Tue Feb 27 16:03:23 2024 +0100

    Delete obselete feed Donkey Republic Barcelona (MobilityData#599)

commit b9d8390
Author: Fabien Richard-Allouard <[email protected]>
Date:   Wed Feb 14 12:54:14 2024 +0100

    Fix geofencing examples (MobilityData#595)

    * Remove overlapping polygon from example

    * Change rule to override global rules in example

commit 6a9150c
Author: Fabien Richard-Allouard <[email protected]>
Date:   Mon Jan 29 14:16:55 2024 +0100

    Fix markdown formatting (MobilityData#596)

    This PR fixes the markdown formatting when governance.md is imported on https://gbfs.org/specification/process/

commit a3d68f6
Author: Fabien Richard-Allouard <[email protected]>
Date:   Thu Jan 25 17:08:37 2024 +0100

    Syncs systems.csv with transport.data.gouv (MobilityData#593)

    * Syncs systems.csv with transport.data.gouv

    * Move public keys to Authentication Info column

    * Update Authentication Info description in README

    * Rephrase Authentication Info description in README

commit 0775ee4
Author: Fabien Richard-Allouard <[email protected]>
Date:   Tue Jan 23 17:39:47 2024 +0100

    Label global_rules field as added in v3.0-RC (MobilityData#594)

    This editorial change adds a missing label under the field `global_rules` to explain that the field was added in v3.0-RC.

commit e9fc5aa
Author: Ortwin Gentz, FutureTap <[email protected]>
Date:   Mon Jan 22 15:39:26 2024 +0100

    gbfs.md: Fix language about iOS links (MobilityData#585)

commit 411b4f4
Author: Fabien Richard-Allouard <[email protected]>
Date:   Mon Jan 15 15:44:59 2024 +0100

    README.md: Add link to v2.3 Release Notes (MobilityData#592)

    This PR adds the link to the [v2.3 Release Notes](https://github.com/MobilityData/gbfs/releases/tag/v2.3) in the Current Version table of the [README.md](https://github.com/MobilityData/gbfs/blob/master/README.md).

commit 32ccae4
Author: Fabien Richard-Allouard <[email protected]>
Date:   Mon Jan 15 15:00:59 2024 +0100

    GBFS Governance Revisions (MobilityData#579)

    * Add release cycles to the governance

    * Link README to governance.md for Release Cycles

    * Simplify MINOR version release line

    * Update stale bot

    * Typo

commit 4e9c72f
Author: Merja Kajava <[email protected]>
Date:   Thu Jan 11 17:49:29 2024 +0200

    Updated location names in systems.csv (MobilityData#590)

commit 43b9be7
Author: Fabien Richard-Allouard <[email protected]>
Date:   Thu Jan 11 14:39:23 2024 +0100

    Add country name when location is empty (MobilityData#589)

commit 3806cdc
Author: Merja Kajava <[email protected]>
Date:   Thu Jan 11 12:37:46 2024 +0200

    Removed nextbike_fo from systems.csv (MobilityData#588)

commit f6a9c56
Author: Fabien Richard-Allouard <[email protected]>
Date:   Wed Jan 10 16:54:59 2024 +0100

    Location formatting recommendation in systems.csv (MobilityData#587)

    * Location formatting recommendation in systems.csv

    This PR adds a recommendation in README.md about the formatting of the system location in systems.csv.

    * Remove country code from formatting recommendation

    * Remove country code from Location column

commit a36a103
Author: Fabien Richard-Allouard <[email protected]>
Date:   Wed Jan 10 14:52:57 2024 +0100

    Update VéloZef (Brest, FR) system_id (MobilityData#586)

    This PR updates the system_id of VéloZef (Brest, FR) to match the system_id from the [feed](https://api.prod.partners-fs37hd8.zoov.eu/gbfs/2.2/brest/en/system_information.json?&key=OGNhZDNjMDQtYTA0Yi00NzU2LWE0MTItOGJlYzE1Y2E4NGEx): `velozef`.

    Thanks to @XioNoX for noticing ❤️
@richfab richfab changed the base branch from master to feat/v3.1-RC May 17, 2024 07:39
@richfab richfab merged commit e5bc31c into MobilityData:feat/v3.1-RC May 17, 2024
1 check failed
richfab added a commit that referenced this pull request May 22, 2024
* manifest.json: include area, country_code #572
* Reservation Price #457
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants