Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Proof of concept: s/defmulti + arguments-checker #451

Closed
wants to merge 1 commit into from

Conversation

frenchy64
Copy link
Contributor

arguments-checker implements idea outlined in #450.

Multimethod wrapping for non-graalVM JVM Clojure only atm.

@w01fe
Copy link
Member

w01fe commented Oct 1, 2022

Interesting, thanks for exploring this direction. I know this is just a draft for now, not sure if you are looking for feedback on the whole thing, or just the arguments-checker piece, or just putting it up for CI?

In the future I would ideally suggest making a proposal before putting up an implementation for a new feature like s/defmulti, but that's of course up to you. For historical context, we had deliberately omitted this one because it wasn't clear there was significant value above and beyond what's provided by s/defmethod, especially given the necessary complexity to implement it.

In any case, I'm open to reconsidering that but to be able to usefully comment on something like this I would really appreciate at some point an explanation of what user need you're trying to address, how you went about solving it and new user-facing syntax, explanation behind tricky implementation details (why did you need to drop to java, how safe is this from a maintenance perspective), etc. Thanks!

@frenchy64
Copy link
Contributor Author

Someone in #malli in clojurians wanted similar functionality for malli and I thought I'd try it in Schema since I'm more comfortable here. I will move this to my repo since this is just exploratory.

@frenchy64 frenchy64 closed this Oct 1, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants