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

Support for self-hosted js-envs Lumo and Planck #179

Merged
merged 1 commit into from
May 4, 2018

Conversation

mfikes
Copy link
Contributor

@mfikes mfikes commented May 3, 2018

Fixes #116

This PR adds the ability to to lein doo lumo and lein doo planck.

Detailed notes:

Self-hosted ClojureScript doesn't require the ClojureScript compile build step (as lumo and planck operate directly on source, compiling in memory internally). So, in theory, while the build step is harmless, it could be omitted. But, it wasn't clear to me how this could be easily accomplished when the auto mode is in effect (because the ClojureScript compiler's watch capability is used). So, this PR does nothing with respect to the build step and preserves the existing logic.

Also, since lumo and planck don't need the results of the build, the (:output-to compiler-opts) final script runner command line argument is conditionally omitted for the self-hosted testing case. (If you look at the way lumo and planck are executed, they are directed to run the test by issuing a require on the (:main compiler-opts).

I tried putting some of the self-hosted stuff into a separate namespace, with defmethods for :lumo and :planck, which would be an appealing approach, but it is difficult to cleanly do this without wanting a circular dependency between doo.core and that new separate namespace. So this PR just keeps it simple and puts the self-hosted support directly in doo.core.

This PR works with the current lumo release.

For planck recently-released version 2.14.0 is required, otherwise two issue arise which could be worked-around in doo but they are really planck defects:

@bensu
Copy link
Owner

bensu commented May 4, 2018

Looks good, thanks! Can you please add the plank version requirements to the README?

@mfikes
Copy link
Contributor Author

mfikes commented May 4, 2018

Added Planck version requirement to README.

@bensu bensu merged commit caf0386 into bensu:master May 4, 2018
@bensu
Copy link
Owner

bensu commented May 4, 2018

Thanks @mfikes! I'll let @miikka cut the next release when he feels it is ready. He has a better grasp on the latest, unreleased work.

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.

Planck
2 participants