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

Add Jurisdictions API spec (read-only endpoints) #593

Merged

Conversation

janedotx
Copy link
Contributor

Explain pull request

See #491 for the discussion and motivations behind this pull request.

Is this a breaking change

  • No, not breaking

Impacted Spec

Which spec(s) will this pull request impact?

  • geography

Additional context

#491

@janedotx janedotx requested a review from a team as a code owner October 28, 2020 19:21
@CLAassistant
Copy link

CLAassistant commented Oct 28, 2020

CLA assistant check
All committers have signed the CLA.

@janedotx janedotx changed the title Add jurisdictions spec (authoring and read-only) Add jurisdictions spec (read-only endpoints) Oct 29, 2020
@marie-x marie-x changed the title Add jurisdictions spec (read-only endpoints) Add Jurisdictions API spec (read-only endpoints) Oct 29, 2020
@schnuerle schnuerle added the Geography Items related to the Geography API label Oct 31, 2020
@schnuerle schnuerle added this to the 1.1.0 milestone Oct 31, 2020
@schnuerle
Copy link
Member

Feedback below from the working group meeting. See full notes here.

Brian Ng presentation.

  • Would like feedback from providers on if this is a burden to ready from agencies along with policy and geography. Note this is already being done by email now in a few cities like LA and Miami-Dade.
  • Ensure it's listed as optional for cities to implement
  • Ensure it's clear that cities serve the API. Groups of nearby cities self organize and decide who will serve it.
  • Ensure it's authenticated. Could be public in the future.
  • Ensure it has a beta designation and link to beta info in spec.

@brianngca can you make sure to sign the CLA for your contribution?

@schnuerle
Copy link
Member

@brianngca thanks for signing the CLA.

@brianngca and @janedotx If you can incorporate the feedback from the WG meeting in the comment above, I will notify the members to review and comment for the 1.1.0 release. It would need to be reviewed in one of the next 2 WG calls as a final community check for the release.

@brianngca
Copy link
Contributor

@schnuerle Thanks for writing up the summary of the WG meeting and feedback. I'll start incorporating the feedback into the text with @janedotx

jurisdiction/README.md Outdated Show resolved Hide resolved
jurisdiction/README.md Outdated Show resolved Hide resolved
jurisdiction/README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
jurisdiction/README.md Outdated Show resolved Hide resolved
jurisdiction/README.md Outdated Show resolved Hide resolved
jurisdiction/README.md Outdated Show resolved Hide resolved
jurisdiction/README.md Outdated Show resolved Hide resolved
jurisdiction/README.md Outdated Show resolved Hide resolved
@schnuerle
Copy link
Member

@janedotx I made some comments to help resolve some formatting, TOC, and beta language issues. All minor. If you can edit the PR and check the recommended "allow edits from maintainers" checkbox I could make these for you.

@schnuerle
Copy link
Member

schnuerle commented Dec 1, 2020

This commit has a lot of what looks like auto-formatting changes going on to the main MDS readme. Some are ok like removing spaces, but others are changing "-" to "*" in lists. While this is ok, I don't think these changes need to be made in the Jurisdiction PR. Could you revert changes like this?

@janedotx
Copy link
Contributor Author

janedotx commented Dec 2, 2020

@janedotx I made some comments to help resolve some formatting, TOC, and beta language issues. All minor. If you can edit the PR and check the recommended "allow edits from maintainers" checkbox I could make these for you.

I can't find the "allow edits from maintainers" checkbox on the lacuna-tech fork.

@janedotx
Copy link
Contributor Author

janedotx commented Dec 2, 2020

This commit has a lot of what looks like auto-formatting changes going on to the main MDS readme. Some are ok like removing spaces, but others are changing "-" to "*" in lists. While this is ok, I don't think these changes need to be made in the Jurisdiction PR. Could you revert changes like this?

So that link just goes to this PR, and looking at the diff, the only addition to the general README is the addition of the Jurisdictions section. Commit b1fb43df732f4a30a2d45b87ac83650391957602 did make a bunch of formatting changes, thanks to my autoformat on save being silly, but commit f536df93019990fe2ead9ff671f4fdd242ea9d50 did revert them. Is that still not okay?

@schnuerle
Copy link
Member

This commit has a lot of what looks like auto-formatting changes going on to the main MDS readme. Some are ok like removing spaces, but others are changing "-" to "*" in lists. While this is ok, I don't think these changes need to be made in the Jurisdiction PR. Could you revert changes like this?

So that link just goes to this PR, and looking at the diff, the only addition to the general README is the addition of the Jurisdictions section. Commit b1fb43df732f4a30a2d45b87ac83650391957602 did make a bunch of formatting changes, thanks to my autoformat on save being silly, but commit f536df93019990fe2ead9ff671f4fdd242ea9d50 did revert them. Is that still not okay?

Yes that's ok, didn't see the cleanup later, thank you for clarifying.

Copy link
Member

@schnuerle schnuerle left a comment

Choose a reason for hiding this comment

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

Looks good for release candidate.

@schnuerle schnuerle merged commit a02c022 into openmobilityfoundation:dev Dec 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Geography Items related to the Geography API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants