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

Upgrade amoc_rest with new openapi gens and enforce OTP27 #5

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

NelsonVides
Copy link
Collaborator

@NelsonVides NelsonVides commented Nov 16, 2024

To merge after esl/amoc_rest#11

After integrating the new generated code, there's a couple of differences, mostly as we enforce OTP27:

  • With json, we don't need any jsx/jiffy/other, and it's also the fastest. The generated code also allow us to distinguish between HTTP calls with content-type: application/json vs others, see the pattern-matching clauses on accept_callback/4.
  • With code:get_doc/1, documentation chunks are now embedded in the module and can be extracted and read at runtime, hence we can rid of docsh.

@NelsonVides NelsonVides force-pushed the openapi_server branch 2 times, most recently from a6c92c2 to e1ebdaf Compare November 16, 2024 15:47
@NelsonVides NelsonVides marked this pull request as ready for review November 16, 2024 16:00
@NelsonVides NelsonVides marked this pull request as draft November 16, 2024 16:00
Comment on lines +16 to +17
get_documentation(Scenario) ->
case code:get_doc(Scenario) of
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is one of my favourite things of upgrading to OTP27, docs are embedded on the module now.

@NelsonVides NelsonVides marked this pull request as ready for review November 27, 2024 07:31
@@ -55,7 +60,8 @@ patch(BaseUrl, Path, Body) ->

-spec request(string(), binary(), binary()) ->
{integer(), json()}.
request(BaseUrl, Path, Method) -> request(BaseUrl, Path, Method, <<"">>).
request(BaseUrl, Path, Method) ->
request(BaseUrl, Path, Method, <<"">>).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
request(BaseUrl, Path, Method, <<"">>).
request(BaseUrl, Path, Method, <<>>).

@@ -42,11 +45,13 @@ put(BaseUrl, Path, Body) ->

-spec patch(string()) ->
{integer(), json()}.
patch(Path) -> patch(get_url(), Path, <<"">>).
patch(Path) ->
patch(get_url(), Path, <<"">>).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
patch(get_url(), Path, <<"">>).
patch(get_url(), Path, <<>>).

Copy link
Contributor

@JanuszJakubiec JanuszJakubiec 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 👍🏻

@JanuszJakubiec JanuszJakubiec merged commit c8712cd into main Nov 27, 2024
2 checks passed
@JanuszJakubiec JanuszJakubiec deleted the openapi_server branch November 27, 2024 09:50
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