Skip to content

[finch] Allow finch server to compile for CI checks#7

Merged
wing328 merged 1 commit intoOpenAPITools:masterfrom
jimschubert:finch_server_compile_fix
May 13, 2018
Merged

[finch] Allow finch server to compile for CI checks#7
wing328 merged 1 commit intoOpenAPITools:masterfrom
jimschubert:finch_server_compile_fix

Conversation

@jimschubert
Copy link
Member

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

Previous error handling implementation had types returning
Either[CommonError, UserType], but implemented with the scala shortcut
??? which throws an exception instead. This causes compilation to fail
with a message that the expected CommonError is of type Any. This is
often fixable with generic upper bounds constraints, but this is
overkill for a placeholder implementation. Returning a temporary 'TODO'
type solves the compile error, and should allow CI to check for valid
compilation on changes.

Included in this is also a fix to support optional query parameter
types. The spec used to generate the finch server has optional query
parameters, but the version of finch in the template doesn't support
options on query parameters. Finch does, however, aggregate everything
(headers, query string, path parameters, etc) under "param" with
"paramOption" for those which are optional types.

Previous error handling implementation had types returning
Either[CommonError, UserType], but implemented with the scala shortcut
??? which throws an exception instead. This causes compilation to fail
with a message that the expected CommonError is of type Any. This is
often fixable with generic upper bounds constraints, but this is
overkill for a placeholder implementation. Returning a temporary 'TODO'
type solves the compile error, and should allow CI to check for valid
compilation on changes.

Included in this is also a fix to support optional query parameter
types. The spec used to generate the finch server has optional query
parameters, but the version of finch in the template doesn't support
options on query parameters. Finch does, however, aggregate everything
(headers, query string, path parameters, etc) under "param" with
"paramOption" for those which are optional types.
@wing328 wing328 added this to the 3.0.0 milestone May 13, 2018
@wing328
Copy link
Member

wing328 commented May 13, 2018

Tested locally and it works.

[info] ScalaTest
[info] Run completed in 47 milliseconds.
[info] Total number of tests run: 0
[info] Suites: completed 0, aborted 0
[info] Tests: succeeded 0, failed 0, canceled 0, ignored 0, pending 0
[info] No tests were executed.
[success] Total time: 1515 s, completed May 13, 2018 11:04:33 AM

@wing328 wing328 merged commit 52322c4 into OpenAPITools:master May 13, 2018
@wing328
Copy link
Member

wing328 commented May 13, 2018

I'll add it to the CI in a separate PR.

@jimschubert jimschubert deleted the finch_server_compile_fix branch May 13, 2018 13:35
Thecrazyskull added a commit to Thecrazyskull/openapi-generator that referenced this pull request Nov 26, 2020
AbstractKotlinCodegen: always prefer parent type regardless of whethe…
padamstx pushed a commit to ibm-devx-sdk/openapi-generator that referenced this pull request Oct 5, 2022
nilskuhn pushed a commit to nilskuhn/openapi-generator that referenced this pull request Apr 6, 2023
Fix "Unable to access jarfile" when path contains spaces
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