Skip to content

Fix query flag casing#19

Closed
jwoudenberg wants to merge 22 commits intojoneshf:masterfrom
NoRedInk:fix-query-flag-casing
Closed

Fix query flag casing#19
jwoudenberg wants to merge 22 commits intojoneshf:masterfrom
NoRedInk:fix-query-flag-casing

Conversation

@jwoudenberg
Copy link
Copy Markdown

No description provided.

stoeffel and others added 22 commits June 11, 2019 14:51
Creates space for other test suites in the `test` directory, such as the
golden test I'm about to add.
These test (and document) what code generation currently looks like for
various Servant API constructors.
Query flags are the Http equivalent of booleans. You can either set them
or you cannot. Currently the generated ruby code treats them as values:
You pass in a string and it gets added to the query string.

This ensures query flags in a servant API are modeled as booleans in the
generated ruby code.
When adding a query flag to an existing endpoint it would be nice if
this were a backwards-compatible change. From the HTTP perspective it
already is: the query flag is an optional element of the URI that can be
omitted. But code generation produces a non-optional argument. This
imposes a hard order-of-deploy between the service that supports the new
parameter and the client that makes use of it (or not).

This changes the code so the query flag is an optional argument in ruby
generated code. That way the backwards-compatibility of adding a query
parameter to an API is preserved into our generated code.
We weren't using the snake-cased variable name for a query flag in all
the places we were using the query flag.
@jwoudenberg jwoudenberg deleted the fix-query-flag-casing branch December 29, 2020 13:29
@jwoudenberg jwoudenberg restored the fix-query-flag-casing branch December 29, 2020 13:30
@jwoudenberg jwoudenberg deleted the fix-query-flag-casing branch December 29, 2020 13:30
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.

2 participants