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

Required strings #600

Open
1 of 3 tasks
tdelmas opened this issue Feb 20, 2024 · 5 comments · May be fixed by #636
Open
1 of 3 tasks

Required strings #600

tdelmas opened this issue Feb 20, 2024 · 5 comments · May be fixed by #636

Comments

@tdelmas
Copy link
Contributor

tdelmas commented Feb 20, 2024

What is the issue and why is it an issue?

REQUIRED Strings : should the specification says they MUST NOT be empty ("") ?

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

The specification could say either:

  • REQUIRED Strings MUST NOT be empty
  • Empty Strings are considered as not present

Is your potential solution a breaking change?

  • Yes
  • No
  • Unsure

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

@richfab
Copy link
Contributor

richfab commented Feb 28, 2024

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

@mobilitydataio
Copy link
Contributor

This discussion has been automatically marked as stale because it has not had recent activity. It will be closed in 30 days if no further activity occurs. Thank you for your contributions.

@tdelmas
Copy link
Contributor Author

tdelmas commented Apr 29, 2024

#600 (comment)

Those examples are for virtual stations, so maybe they should be parking areas (geofencing zones)?

@tdelmas
Copy link
Contributor Author

tdelmas commented May 13, 2024

Before opening a PR, I would like the opinion of the community:

  • Should the specification forbid empty string for REQUIRED string fields ?
  • Should the specification allow empty string for OPTIONAL string fields ?

Please give your feedback with 👍 👎 👀 :

  • 👍 Completely FORBID empty strings
  • 👀 Only FORBID empty string for REQUIRED strings
  • 👎 ALLOW empty string even for REQUIRED strings (current situation)

My view: if a REQUIRED field should accept empty strings, maybe it shouldn't be REQUIRED.

@leonardehrenfried
Copy link
Contributor

leonardehrenfried commented May 13, 2024

I agree with you I would argue that an empty string is never something you should put in a GBFS feed. It's either required or optional but empty is string kind of both, which seems like a modelling error.

tdelmas added a commit to tdelmas/gbfs that referenced this issue May 16, 2024
@tdelmas tdelmas linked a pull request May 16, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants