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.

Sign up for GitHub

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

Jump to bottom

Explicit field hierarchy and array sub-types #583

Merged
merged 8 commits into from
Jan 4, 2024
Merged

Explicit field hierarchy and array sub-types #583

merged 8 commits into from
Jan 4, 2024

Conversation

richfab
Copy link
Contributor

@richfab richfab commented Jan 4, 2024

Initial PR: #576

Problem

  • The field hierarchy with dashes is difficult to read.
  • The description of required fields inside optional objects is too verbose.
  • The type Array is vague.

Solution

This PR makes the field hierarchy and the array sub-types explicit:

Before After
image Screenshot 2024-01-04 at 11 02 55

The changes are editorial only. A vote is not required.

richfab and others added 4 commits January 4, 2024 09:13
@richfab @tdelmas
Improve readability (#582)
acf0e6a 
* Improve readability

* replace < and > with &lt; and &gt; for display.

* all fields have a definition

---------

Co-authored-by: Tom <1955774+tdelmas@users.noreply.github.com>
@richfab
Minor rephrasings
3105c64
@richfab
Remove duplicate of Object definition
5621f32
@richfab
Make field hierarchy explicit
a577895
@richfab richfab mentioned this pull request Jan 4, 2024
Closed
3 tasks
tdelmas
tdelmas suggested changes Jan 4, 2024
View reviewed changes
Copy link
Contributor

@tdelmas tdelmas left a comment

Choose a reason for hiding this comment

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

Arrays of IDs should be defined as such. (descriptions may need to be updated)

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
gbfs.md Outdated Show resolved Hide resolved
gbfs.md Outdated Show resolved Hide resolved
@richfab
Copy link
Contributor Author

richfab commented Jan 4, 2024

Great catch for Array<ID> @tdelmas! Thanks!

@richfab richfab requested a review from tdelmas January 4, 2024 11:19
josee-sabourin
josee-sabourin reviewed Jan 4, 2024
View reviewed changes
gbfs.md Outdated Show resolved Hide resolved
gbfs.md Show resolved Hide resolved
gbfs.md Show resolved Hide resolved
gbfs.md Show resolved Hide resolved
gbfs.md Show resolved Hide resolved
@richfab richfab requested a review from josee-sabourin January 4, 2024 14:32
@richfab richfab merged commit 212386e into master Jan 4, 2024
1 check passed
@richfab richfab deleted the clarify-types branch January 4, 2024 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tdelmas tdelmas tdelmas approved these changes

@futuretap futuretap futuretap left review comments

@josee-sabourin josee-sabourin josee-sabourin approved these changes

@isabelle-dr isabelle-dr Awaiting requested review from isabelle-dr

@davidgamez davidgamez Awaiting requested review from davidgamez

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@richfab @futuretap @tdelmas @josee-sabourin