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 response specs #6

Merged
merged 11 commits into from
Aug 22, 2024
Merged

Conversation

bombaywalla
Copy link
Contributor

Added support for generating response specs in addition to parameter specs.
With rudimentary tests.

If there is no content, just return a nil? schema without any other
processing.

If the media type is "default" they convert to a keyword, otherwsie
leave it as a string.  This is how Reitit wants it.
@lispyclouds
Copy link
Owner

Hey hey @bombaywalla long time! Thanks a lot for this, will take a proper look and get back asap.

src/navi/core.clj Outdated Show resolved Hide resolved
src/navi/core.clj Outdated Show resolved Hide resolved
src/navi/core.clj Outdated Show resolved Hide resolved
src/navi/core.clj Outdated Show resolved Hide resolved
@bombaywalla
Copy link
Contributor Author

Thanks for the feedback.
I've pushed a new commit with the changes discussed.

@lispyclouds
Copy link
Owner

You could also take out the response coercions from the unsupported section on the readme too in this PR :)

Since a LinkedHashMap implements HashMap, a LinkedHashMap "just works"
with Clojure, without any interop needed.  So, re-wrote it using
reduce-kv. Changed the position of the map parameter from last to
first in order to be consistent with update-keys and update-vals.

Refactored other code to use update-kvs.

Some minor comment changes.
@bombaywalla
Copy link
Contributor Author

bombaywalla commented Aug 22, 2024

Inspired by your experimentations with LinkedHashMap, I found that since a LinkedHashMap also implements HashMap, Clojure "just works" with it without any interop needed. So I re-wrote linked-hash-ma->clj-map to just use reduce-kv and since it is no longer interop-dependent, I renamed it to update-kvs and changed the parameter ordering to keep the map first, like update-keys and update-vals.

I also refactored the other code to use update-kvs. And I moved the definition of update-kvs to earlier in the file.

I updated the README to reflect current reality.

See what you think of the current state and let me know if it can be made better.

Thanks!

src/navi/core.clj Outdated Show resolved Hide resolved
@lispyclouds
Copy link
Owner

Thanks a lot for the efforts and it looks good! Lemme know what you think of my extra change and post that we should be good to merge.

@lispyclouds lispyclouds merged commit 8dd2c18 into lispyclouds:main Aug 22, 2024
@bombaywalla bombaywalla deleted the add-response-specs branch August 23, 2024 23:52
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