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

GBFS documentation versioning and and feed conformance #188

Merged
merged 7 commits into from
Nov 25, 2019

Conversation

antrim
Copy link
Contributor

@antrim antrim commented Nov 6, 2019

Issue #15 contains a discussion thread about versioning GBFS documentation and data. Versioning is needed to enable backwards-compatibility breaking changes.

This PR:

  • Defines a semantic versioning approach for backwards-compatibility breaking (MAJOR) and backwards-compatible (MINOR) GBFS changes
  • Includes release practice guidelines, including a long-term support (LTS) version and maximum number of MAJOR releases per year. New versions would be subject to community vote, so these are guidelines, not hard rules.
  • Defines a backwards-compatible lookup mechanism to disseminate GBFS data feeds encoded in various versions of the specification.
  • Does not yet include the versionCode concept (integers associated with each named version), but this could be added if requested.

Please comment and propose any changes. @MobilityData proposes to bring this to vote in 1 week or less so that GBFS can begin to incorporate necessary changes that break backwards-compatibility.

README.md Outdated

### Version Release Cycles
* There is no strict limitation on the frequency of MAJOR releases, but the GBFS community aims to limit the MAJOR releases to 2 or less every 12 months. To limit releases, breaking changes can be batched together.
* MINOR changes may be applied at any time. There is no guideline to limit the number of MINOR changes.
Copy link
Member

Choose a reason for hiding this comment

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

Are minor changes batched too, or does every change that qualifies as a minor change bump the MINOR version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was intentionally silent so that MINOR changes could be batched or not batched, depending on their timeline. My assumption is that if changes occur soon after each other, we might as well batch. But, with backwards-compatibility issues absent, it doesn't make sense to hold back changes too long for a batched version bump. I'll explicitly state this.

Copy link
Member

Choose a reason for hiding this comment

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

I would state this explicitly just so there isn't any confusion.

README.md Outdated Show resolved Hide resolved
gbfs.md Outdated Show resolved Hide resolved
gbfs.md Outdated Show resolved Hide resolved
gbfs.md Outdated Show resolved Hide resolved
gbfs.md Outdated Show resolved Hide resolved
gbfs.md Outdated Show resolved Hide resolved
gbfs.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
gbfs.md Outdated
}
]
},
"fr" : {
"feeds": [
{
"name": "system_information",
"url": "https://www.example.com/gbfs/fr/system_information"
"url": "https://www.example.com/gbfs/1.0/fr/system_information"
Copy link
Member

Choose a reason for hiding this comment

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

Are we going to remain silent on how versioning should be handled in the URL path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think? Should we offer a recommendation? As this is written, it's up to the discretion of the feed producers. The examples offer suggestions.

Copy link
Contributor

@jcn jcn Nov 7, 2019

Choose a reason for hiding this comment

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

Perhaps we should state that a URL must never change its major version. For example, if a feed doesn't want to include a version in the path, that's fine, but it must not change from v1 to v2 ever.

This actually brings up another question - can a feed change from 1.0 to 1.1? Technically this should not break anything, and it would mean that publishers could have, say, https://www.example.com/gbfs/1/fr/system_information for their feed that conformed to the 1.X spec. Would this be confusing for consumers?

Copy link
Member

Choose a reason for hiding this comment

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

One challenge for dictating versions in URL paths is that it could conflict with existing versioning schemes or numbers used by producers in URL paths. For example, HOPR currently uses integers in the URL path to identify systems - https://gbfs.hopr.city/api/gbfs/6 vs https://gbfs.hopr.city/api/gbfs/8.

So there could be significant backwards compatibility concerns.

If we require the auto-discovery file (see proposal at #189), I'm inclined to let producers manage their own URL paths.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jcn I just pushed a change that responds to what you pointed out. Please let me know what you think…

gbfs.md Outdated Show resolved Hide resolved
gbfs.md Outdated Show resolved Hide resolved
@barbeau
Copy link
Member

barbeau commented Nov 6, 2019

Thanks @antrim! Looks pretty good, I suggested some changes.

Copy link
Contributor

@heidiguenin heidiguenin left a comment

Choose a reason for hiding this comment

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

Reviewing after @barbeau means lots of thumbs-up responses from me. Looking good!

README.md Outdated Show resolved Hide resolved
In response to review by @barbeau
@antrim antrim changed the title GBFS documentation versioning and data encoding practice GBFS documentation versioning and and feed conformance Nov 7, 2019
Copy link
Contributor Author

@antrim antrim left a comment

Choose a reason for hiding this comment

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

Thanks for your review, @barbeau and @heidiguenin. I've modified the proposal according to most of the suggestions. There is one outstanding question around URL paths and versions.

Proposed next steps

  • Let's keep this open until Tuesday (UTC) of next week for review and comment.
  • And then call a vote on Wednesday (UTC). Assuming there is support for this proposal, I propose that we also call a vote on changes that break backwards-compatibility at the same time, preparing to implement the versioning process.

@jcn
Copy link
Contributor

jcn commented Nov 7, 2019

A question about the transition from our unversioned to versioned world: should we just state that all unversioned feeds should be considered 1.0? The current spec is considered an implicit 1.0 anyway, so this would help in terms of moving forward, and for consumers of feeds that have not upgraded yet.

README.md Outdated
@@ -40,6 +40,26 @@ The general outline for changing the spec has 4 steps:
3. Find at least one GBFS producer to implement and test the proposed change.
4. Submit a final request-for-comments on the proposed change to the issue discussion. If no outstanding issues are identified after one week’s time, and there is general agreement that the proposed change is worthwhile and follows the GBFS guiding principles outlined below, the proposal will be officially adopted.

## Specification Versioning
To enable the evolution of GBFS, including changes that would otherwise break backwards-compatibility with consuming applications, GBFS documentation is versioned. A git tag in the form of `X.Y` establishes semantic versions. Multiple changes (commits) may be batched into a single new release.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that git tags technically have to start with a letter, so the tag will be vX.Y where the version name itself will be X.Y

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

_GBFS producers_ should provide endpoints that conform to both the current specification long term support (LTS) branch as well as the latest release branch within at least 3 months of a new spec `MAJOR` or `MINOR` version release. See [specification versioning](README.md#specification-versioning)

_GBFS consumers_ should, at a minumum, support the current LTS branch. It highly recommended that GBFS consumers support later releases.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this "should" or "must?" If a producer does not provide LTS and the latest feed, are there any repercussions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is framed as a "Best Practice" to work within the bounds of the authority of specification authors. This is a recommendation, not a requirement. Making any sort of requirement with repercussions is the domain of cities and other governments and/or bikeshare contract holders.

Copy link
Contributor

Choose a reason for hiding this comment

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

A clarifying question: what does "It highly recommended that GBFS consumers support later releases." mean?

## Files
This specification defines the following files along with their associated content:

File Name | Required | Defines
--------------------------- | ----------------------- | ----------
gbfs.json | Optional | Auto-discovery file that links to all of the other files published by the system. This file is optional, but highly recommended.
gbfs_versions.json | Optional | Lists all feed endpoints published according to versions of the GBFS documentation.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little confused about where this sits. Does this file get published for every version? i.e. does this file appear in 1.0, 1.1 and 1.2 locations? Regardless, I propose we make it required in order to be able to find all of the relevant feeds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file would sit in all the versions.

Proposal: Let's keep this optional for its incorporation into a "1.0" version. Technically, the change is backwards-compatible if we make the file optional.

We could make this required in a future backwards compatibility breaking MAJOR change.

Copy link
Contributor

Choose a reason for hiding this comment

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

We could make this required in a future backwards compatibility breaking MAJOR change.

Let's update this field to indicate that it will probably become required in a future version so producers are encouraged to start publishing now.

gbfs.md Outdated
Example:
```json
{
"last_updated": 1434054678,
"ttl": 3600,
"version":"1.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency with the rest of the documentation, wherever new JSON fields were added, can we add a space after the :?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Changed.

gbfs.md Outdated
"ttl": 0,
"version":"1.0",
"data": {
[
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be

"versions": [

(you're missing the key name)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Fixed.

antrim and others added 2 commits November 18, 2019 10:58
"version": "2.1" -> "version": "1.0" as per @jcn suggestion

Co-Authored-By: Jesse Chan-Norris <[email protected]>
Change example data for gbfs.json to be consistent with a v1.0 example
@antrim
Copy link
Contributor Author

antrim commented Nov 21, 2019

Voting has closed. Both Lime and Transit voted for the change. No organizations voted against.

Current GBFS governing rules say that at least one producer and consumer have to implement the change before it is adopted, I propose that we merge to the master branch (if there are no objections) for these reasons:

  • The change includes new governance rules.
  • An agreed upon versioning scheme is a prerequisite for some of the proposed changes that would break backwards-compatibility. Deciding on a versioning scheme now will allow GBFS stakeholders to better plan.
  • This change change itself is backwards compatible. Consuming software would not need to be changed until a new version is released.

Does anyone have objections to merging into the master branch?

@jcn
Copy link
Contributor

jcn commented Nov 21, 2019

I am in favor of this change and in merging it at the end of the voting period but for the sake of process we should wait for the actual end as per your original call for a vote.

I hereby call a vote on this proposal. Voting will be open for 7 full days, until 11:59PM UTC on Friday, Nov 22.

@antrim
Copy link
Contributor Author

antrim commented Nov 21, 2019

Oops. Sorry about that confusion with the dates. Voting is of course open through Friday.

@yocontra
Copy link
Contributor

+1 for Stae

@tmontes
Copy link

tmontes commented Nov 21, 2019

May I suggest that if this PR is merged, as it seems it will, then the repository is tagged as soon as possible: we should have, I suppose, at least a version 1.0 tag that pinpoints which of the more than 20 commits claiming to be version 1.0 in their version histories is actually version 1.0. :-)

Thanks!

@tmontes
Copy link

tmontes commented Nov 21, 2019

PS: For what it's worth, as an independent consultant working on an MDS/GBFS related project, I'm in favour of the proposed changes.

@evansiroky
Copy link
Contributor

+1 from IBI Group.

@mplsmitch
Copy link
Collaborator

+1

1 similar comment
@sven4all
Copy link
Contributor

+1

@antrim
Copy link
Contributor Author

antrim commented Nov 25, 2019

The change passes! We had 6 votes in favor from consumers or operators and 0 votes against:

  • Consumers (4): Transit, IBI Group, Openbike, Stae
  • Operators / producers (2): Lime, Motivate
  • Unaffiliated (2) - not counted

Next steps:

  • I'll merge the PR.
  • We'll begin to implement the versioning scheme by tagging releases. There are a number of PRs open to voting or soon to be, so this is timely!

@antrim antrim merged commit 44c8688 into master Nov 25, 2019
@antrim antrim deleted the versioning-proposal branch November 25, 2019 21:36
antrim added a commit that referenced this pull request Dec 18, 2019
@antrim antrim added proposal:nonbreaking v1.1 Candidate change for GBFS v1.1 and removed proposal:breaking labels Jan 3, 2020
antrim added a commit that referenced this pull request Jan 27, 2020
fixes the merge of PR #188
addes back section "Files"
@heidiguenin
Copy link
Contributor

We'd love to make this an official part of the spec, but first we need to see this change being implemented. Could you comment here if your organization has implemented this?

@johnpena @gcamp @contra @evansiroky @sven4all ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.