Forbid empty Strings

[tdelmas]

Fix #600

See also https://mobilitydata-io.slack.com/archives/CNXA9ASBV/p1715598631556219

What is the proposal?

Specify that strings (including IDs) SHOULD NOT be empty. (And maybe at a later time a separate PR with breaking change with specify MUST NOT)

Is this a breaking change?

• [ ] Yes
• [X] No
• [ ] Unsure

Which files are affected by this change?

All of them (because of the IDs)

Examples of affected feeds

https://mobilitydata-io.slack.com/archives/CNXA9ASBV/p1708449123435399?thread_ts=1708429919.754579&cid=CNXA9ASBV

Examples for feeds currently using (very few) empty station names:

• BOLT Stuttgart (republished by MobiData BW via Lamassu, as original feed is access restricted)
• Bird Sarreguemines

[tdelmas]

Before submitting to a vote, do you think that change could be in 3.2 ? It is technically a breaking change, but I think it worth it to improve the GBFSv3. Thoughts?

[richfab]

Hi @tdelmas,

I think we should avoid making an exception to the change process.

If the community wants to make the change in v3.2-RC, we could consider:

1. Making the change non-breaking with "SHOULD not be empty" instead of "MUST not be empty".
2. Asking producers to fill in the missing values.

[tdelmas]

@richfab Do you think the update I pushed is compatible with the next v3.X-RC? Do you have an opinion about it?

[mplsmitch]

I'm more inclined to support this as a requirement (MUST) in the next major version. I'd like to hear your reasoning for this interim step. I think using optional SHOULD in place of MUST to get it in a point release sooner is unlikely to result in changes to existing data sets. We have lots of optional fields with SHOULD in their definitions and few of them show up in feeds. I also think this would be a whole lot cleaner if you only changed the String field type definition (and possibly Localized String) and left all the other fields definitions that contain strings as they are.

[tdelmas]

@mplsmitch thank you for the feedback.

I'd like to hear your reasoning for this interim step.

• "MUST" has its place in the next major version
• In the interim, it's important to;
  • Avoid more feeds with empty strings
  • Encourage current feeds with empty strings to remove them
  • Or discover corner-cases that we missed where empty strings are indeed useful

That's why I think we should push the "SHOULD" as soon as possible.

But if the community wants to release a major version soon, maybe that step is not necessary?

I also think this would be a whole lot cleaner if you only changed the String field type definition

That's what I did with the last commit (I only changed string into String for IDs to emphasize they refer to that definition).

(and possibly Localized String)

I don't think it is necessary at the Localized String definition references the String one.

[mplsmitch]

But if the community wants to release a major version soon, maybe that step is not necessary?

Release timing is limited by the Governance which states: "MAJOR releases are limited to no more than once a 12 month period. "

So I think the earliest this could happen is 1 yr from the release of v3.0 meaning spring of 2025.

A minor release could happen in fall of 2024 but the next step is getting through a vote.

Releases typically contain a number of changes. We have never done a release based on a single change. Right now there are no PRs that have been put to a vote for the next release.

[richfab]

I think that waiting for the next MAJOR release (with "MUST not be empty") is reasonable for this change.

On the other hand, @tdelmas you are welcome to open a vote if you think it is useful to try to pass this change in a MINOR version (with "SHOULD not be empty").

Thank you @tdelmas and @mplsmitch for your involvement in this issue 🙏

[tdelmas]

I hereby call a vote on this proposal. Voting will be open for 10 full calendar days until 11:59PM UTC on Sunday, September 22, 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.

[futuretap]

+1 from Where To? / FutureTap.

[testower]

Assuming we are now voting for the MUST-variant of the proposal: +1 from Entur

[cmonagle]

+1 from Transit

[leonardehrenfried]

+1 OpenTripPlanner

In practise this is already implemented, since we reject entities that have empty names for example.

Although I'm not sure if we are voting for the MUST on the next major version or a SHOULD.

[tdelmas]

@leonardehrenfried This is a vote for "Specify that strings (including IDs) MUST NOT be empty." (breaking change, so for the next major)

[richfab]

🗳️ Voting on this Pull Request closes in 2 calendar days (11:59PM UTC on September 22). 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. Thank you 🙏

[richfab]

This vote has now closed, and it passes!

Votes in favor:

• Where To? / FutureTap (consumer)
• Entur (producer)
• Transit (consumer)
• OpenTripPlanner (consumer)

There were no votes against.

This change will be part of v4.0-RC, planned to be released when there will be enough breaking changes to constitute a new MAJOR version (likely in Nov 2025), as per the version release cycle in the governance.

Thank you for your involvement in the GBFS spec 🙏