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

[mds-api-server] [ALL VERSIONED APIS] Feature/neil/add versioning options #332

Merged
merged 5 commits into from
May 18, 2020

Conversation

avatarneil
Copy link

Update the API Versioning middleware to support OPTIONS requests for version negotiation as per https://github.com/openmobilityfoundation/mobility-data-specification/blob/master/general-information.md

PR Checklist

  • simple searchable title - [mds-db] Add PG env var, [config] Fix eslint config
  • briefly describe the changes in this PR
  • mark as draft if should not be merged
  • write tests for all new functionality

Impacts

  • Provider
  • Agency
  • Audit
  • Policy
  • Compliance
  • Daily
  • Native
  • Policy Author

@avatarneil avatarneil self-assigned this May 18, 2020
* If the client did not negotiate a valid version, fall-through to a 406 response.
*/
if (supported.length > 0) {
const [{ latest }] = supported.sort((a, b) => b.q - a.q)

Choose a reason for hiding this comment

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

Nit but it seems to me that you could factor this out of the two branches of this if statement.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah I was thinking about that too, there's not a super clean way to structure this logic IMO. On one hand, being concise is ideal (though you can't be very concise here), on the other hand, being expressive is ideal. I'm gonna think on this a bit before merging and see if I can find a really clean way of writing it.

Choose a reason for hiding this comment

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

IMHO "expressive" >> "concise"

@avatarneil avatarneil merged commit a69c795 into develop May 18, 2020
@avatarneil avatarneil deleted the feature/neil/add-versioning-options branch May 18, 2020 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants