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

incorrect use of Clojure :arglists metadata? #831

Closed
stuarthalloway opened this issue Jul 10, 2015 · 7 comments
Closed

incorrect use of Clojure :arglists metadata? #831

stuarthalloway opened this issue Jul 10, 2015 · 7 comments

Comments

@stuarthalloway
Copy link

The proposed fix to Clojure bug http://dev.clojure.org/jira/browse/CLJ-1232 appears to break hystrix-clj, which (if the ticket comments are correct) uses arglists metadata in a way different from the Clojure compiler.

At first glance this seems to be a bug in hystrix-clj that will become breaking once http://dev.clojure.org/jira/browse/CLJ-1232 is applied. Looking for input/context before we do that.

@benjchristensen
Copy link
Contributor

Thanks @stuarthalloway for the heads up on this.

@daveray Can you help look into this?

@daveray
Copy link
Contributor

daveray commented Jul 20, 2015

Sorry for the slow response. Vacation + sickness. I'll take a look this week.

@mattrjacobs
Copy link
Contributor

@daveray Are you still able to take a look?

daveray added a commit to daveray/Hystrix that referenced this issue Aug 6, 2015
This is a fix for issue Netflix#831 (Netflix#831) which
is a blocker for http://dev.clojure.org/jira/browse/CLJ-1232. Instead of generating
:arglists as (list (quote [..])*), generate (quote ([..]*)). Tests still
pass and (doc command) works as intended. I have not directly tested
this with Clojure patched with CLJ-1232.
@daveray
Copy link
Contributor

daveray commented Aug 6, 2015

Hi. @stuarthalloway there's a proposed fix here: daveray@11d5bad
The only reason for the generated :arglists was to make (doc command) look nice. Hope this helps. Sorry about the delay.

@mattrjacobs
Copy link
Contributor

Thanks @daveray. I don't know enough Clojure to evaluate the fix - are you comfortable with me merging and releasing, or would you prefer to wait until some validation can be done?

@daveray
Copy link
Contributor

daveray commented Aug 6, 2015

@mattrjacobs I'm comfortable with merge and release.

This was referenced Aug 6, 2015
@mattrjacobs
Copy link
Contributor

Merged in #852

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants