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

WIP add support for Babashka #128

Closed
wants to merge 1 commit into from

Conversation

AndreTheHunter
Copy link

@AndreTheHunter AndreTheHunter commented Jun 25, 2020

Summary

#113

  • added :bb specific reader conditionals
  • made expound an optional dependency
  • made cljc.java-time a required dependency
  • combined optionals.clj and .cljs into .cljc
  • combined platform.clj and .cljs into .cljc
  • created bb-test.sh and bb_runner.cljc for running tests in Babashka
  • a lot of simplification and cleanup

TODO

Outstanding changes are marked as FIXME

  • update Babashka to support missing features WIP add support for Babashka #128
  • fix cljs tests (currently failing on `master)
  • fix clj yaml test (currently failing on `master)
  • fix clj spec test
  • test installed artifact (compiled .jar) in Babashka
  • add yaml support
  • add usage example
  • update Readme

Outstanding changes are marked as FIXME

- added :bb specific reader conditionals
- made `expound` an optional dependency
- made `cljc.java-time` a required dependency
- combined optionals.clj and .cljs into .cljc
- combined platform.clj and .cljs into .cljc
- created `bb-test.sh` and `bb_runner.cljc` to running tests in
   Babashka
- a lot of simplification and cleanup
@AndreTheHunter AndreTheHunter changed the title WIP add support for Babashka #113 WIP add support for Babashka Jun 25, 2020
@AndreTheHunter AndreTheHunter changed the title #113 WIP add support for Babashka WIP add support for Babashka - fix-113 Jun 25, 2020
@AndreTheHunter AndreTheHunter changed the title WIP add support for Babashka - fix-113 WIP add support for Babashka Jun 25, 2020
@AndreTheHunter
Copy link
Author

Phew, that was quite the rampage. Happy to answer any questions or discuss any changes/improvements. Feel free to jump in since "Allow edits and access to secrets by maintainers" is on. @l3nz

@l3nz l3nz marked this pull request as ready for review June 25, 2020 13:02
@l3nz
Copy link
Owner

l3nz commented Jun 25, 2020

Sorry I clicked on wrong button. :-)

Couple of comments:

  • I would not want a conditional for fdef, it is really polluting the code. Can we somehow inject a dummy do-nothing fdef into the spec, maybe with a conditional?
  • IIRC, I think that expound is used somewhere to generate error messages
  • what are things that need upstream (bb) support? I'd rather ship a feture less than depend on very latest

In general, good job! very torough. Having bb would be major (and being included in bb even more :-) )

@AndreTheHunter AndreTheHunter marked this pull request as draft July 2, 2020 12:01
@l3nz
Copy link
Owner

l3nz commented Feb 21, 2022

Closing because of #145

@l3nz l3nz closed this Feb 21, 2022
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