-
Notifications
You must be signed in to change notification settings - Fork 0
Add bb tasks and update deps #1
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
base: main
Are you sure you want to change the base?
Conversation
1c84022
to
a402960
Compare
a402960
to
1fdb0f6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing contribution! Thanks for your effort.
I left a comment to change the Type
used in a test since it's currently preventing that step to progress.
src/app/tutorial.cljs
Outdated
@@ -1,7 +1,7 @@ | |||
(ns app.tutorial | |||
(:require | |||
[clojure.string :as string] | |||
[sci.impl.vars :refer [SciVar]])) | |||
[sci.impl.vars :refer [IVar]])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the new version of sci SciVar
has been removed, IVar
is something else.
I found that here we can require instead [sci.lang :refer [Var]]
and use it for testing afterwards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eliascotto Thank you for the correction - updated.
src/app/tutorial.cljs
Outdated
Create a global variable called `foo` with a value. E.g. `(def foo \"bar\")`" | ||
:test #(and (instance? SciVar %) (= "foo" (-> (.-meta %) :name str)))} | ||
:test #(and (instance? IVar %) (= "foo" (-> (.-meta %) :name str)))} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here the test should now check for Var instead (instance? Var %)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eliascotto Thank you for the correction - updated.
93d22e5
to
6f2c309
Compare
Add bb tasks and update deps