medley icon indicating copy to clipboard operation
medley copied to clipboard

Add cljr support

Open E-A-Griffin opened this issue 3 years ago • 3 comments

Adds ClojureCLR support for source/test files, packaging/dependency management, and CI testing.

E-A-Griffin avatar Jul 19 '22 21:07 E-A-Griffin

I wonder if, given the similarity between the clj and cljr branches, instead of listing both, you should use :default as the catch all.

NoahTheDuke avatar Jul 19 '22 22:07 NoahTheDuke

I had considered using :default where there was common overlap between :clj and :cljr but I wasn't sure what best practice was. If @weavejester prefers :default for overlap between clj and cljr then I can certainly make those changes.

E-A-Griffin avatar Jul 19 '22 23:07 E-A-Griffin

I had considered using :default where there was common overlap between :clj and :cljr but I wasn't sure what best practice was. If @weavejester prefers :default for overlap between clj and cljr then I can certainly make those changes.

The reader conditionals page on the Clojure website seems to suggest that :default is designed to be a fallback, and therefore it would seem like the best choice is to be explicit and use :clj and :cljr even in cases where the code is identical.

weavejester avatar Jul 20 '22 02:07 weavejester

Hey there, sorry for the long wait. I've only recently been able to get back to reviewing Clojure PRs and I've been working my way through the list. I notice that in this PR the csproj and sln file seem to have been copied rather than moved? Other than that it looks fine.

weavejester avatar Oct 06 '22 16:10 weavejester

Hey there, sorry for the long wait. I've only recently been able to get back to reviewing Clojure PRs and I've been working my way through the list. I notice that in this PR the csproj and sln file seem to have been copied rather than moved? Other than that it looks fine.

I must not have pushed the local change removing those files. Everything should be good now, thanks for the re-review!

E-A-Griffin avatar Oct 15 '22 14:10 E-A-Griffin

Again, apologies for letting this lie for a while. I've been hesitant to merge this, because in doing so I obviously commit to maintaining an additional language target for Medley, and I've been unwilling to commit one way or the other. On the one hand your work is excellent; on the other I don't think I can reasonably maintain this.

After thinking it through, my thought is that I don't merge this into Medley, and instead I ask you to set up a separate repository for it. I can link to the new repo through Medley's README, so people can easily find the CLR version, but it wouldn't be something I'd maintain or work on. Again, I really appreciate the work you've put into this, but I don't think I'm the person to maintain it.

weavejester avatar Nov 30 '22 16:11 weavejester

No worries on the wait. Yeah, I understand the position you're in, especially given the volume of popular repositories you maintain for the Clojure community as it is. It does make me a bit sad to read, but logistically I get it.

If that's the course of action you want to take, then maybe just link to the current fork of medley I have, i.e. https://github.com/E-A-Griffin/medley, in the README and I can maintain that.

E-A-Griffin avatar Nov 30 '22 21:11 E-A-Griffin

Thank you for understanding. I've added a link to the README to your repository.

weavejester avatar Dec 10 '22 17:12 weavejester