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.

Sign up for GitHub

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

Jump to bottom

Support tracing in ClojureScript #308

Closed
WilliamParker opened this issue Jun 13, 2017 · 7 comments
Closed

Support tracing in ClojureScript #308

WilliamParker opened this issue Jun 13, 2017 · 7 comments
Labels
clojurescript enhancement
Milestone
0.15.1

Comments

@WilliamParker
Copy link
Collaborator

Similarly to #307 I don't see a reason why tracing needs to be Clojure-only. This is mostly, at least in the usage I'm aware of, a low-level tool for debugging and diagnosing performance problems, so it is probably a lower priority than #307, and for my part I'm more likely to work on #307 since I have more use for that. If anyone has need of session tracing in ClojureScript a PR would be welcome though.
@alex-dixon
Copy link
Contributor

Precept has listeners which is copied from Clara's tracing. It's a CLJC file and is used successfully in CLJS.

I just tried converting tracing to .cljc and it broke this test. Interestingly, _ (println bindings matches) on line 180 fixed it for me but I wasn't able to find a real solution.

We may not be able to directly port existing CLJ tests for tracing to CLJC because they use dsl/parse-rule and dsl/parse-query, which are CLJ-only.

@WilliamParker
Copy link
Collaborator Author

@alex-dixon If you push a branch with the uplift you did I can take a look at why it might have failed. Regarding the testing, I'd anticipate uplifting the tracing tests to use def-rules-test as part of this so that they can be shared between Clojure and ClojureScript, similar to the uplift to test-accumulators in #312. def-rules-test was added for issue #296 and there is discussion of it there.

@alex-dixon
Copy link
Contributor

@WilliamParker Thanks. Will do. I actually noticed in the PR I have open that tests were failing locally before I made any changes. Will check again. Thanks for the tip with def-rules-test.

@WilliamParker
Copy link
Collaborator Author

I merged the PR. Would it be helpful to have a release with this and the schema upgrade in it? I'm traveling right now but I can plan on cutting a release sometime next week or the week after that when I'm back unless someone else gets to it first. I think the appropriate versioning for such a release would be 0.15.1 but we can discuss if anyone disagrees; it isn't something I feel strongly about.

@rbrush
Copy link
Contributor

rbrush commented Jun 30, 2017

Agreed that 0.15.1 makes sense. I'm happy to do the release since Will is traveling right now.

@alex-dixon
Copy link
Contributor

Ready any time. No rush or hurry. Thank you :)

@rbrush rbrush added this to the 0.15.1 milestone Jul 2, 2017
@rbrush
Copy link
Contributor

rbrush commented Jul 2, 2017

Closing this since @WilliamParker had merged the PR and preparing to include in 0.15.1.

@rbrush rbrush closed this as completed Jul 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
clojurescript enhancement
Projects
None yet
Milestone
0.15.1
Development

No branches or pull requests
3 participants
@rbrush @WilliamParker @alex-dixon