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

Make build-graph recur until all transitive deps are analyzed #381

Merged
merged 28 commits into from
Jan 19, 2023

Conversation

philomates
Copy link
Contributor

@philomates philomates commented Jan 6, 2023

Until now Clerk did not analyze the full transitive dependency graph which could lead to Clerk not detecting a change properly. Analysis is now recursive which means it's taking a bit longer initially. We cache analysis results per file in memory so subsequent analysis should be fast. We will follow up with visualizing the progress of analysis & execution.

Also discovered cases where classes instead of symbols could end up in the dependency graph and introduced normalization to symbols.

This also gets rid of the ->hash must be ifn? warning which fixes #375.

@mk
Copy link
Member

mk commented Jan 9, 2023

Thanks for looking into this.

I don't understand why the warning is there. Maybe it can just be removed?

To compute a hash that changes when a form or a dependency changes, we must have a hash for each of its dependencies. The warning exists for when that's not the case so it is something that actually needs to be fixed.

@mk mk changed the title seed :->hash with initial value to prevent " ->hash must be ifn?" warning Make build-graph recur until all transitive deps are analyzed Jan 12, 2023
@mk mk force-pushed the fix-hash-warning branch from 03be599 to a632051 Compare January 12, 2023 09:53
@mk mk force-pushed the fix-hash-warning branch from a632051 to e32ad46 Compare January 12, 2023 09:57
@borkdude
Copy link
Collaborator

borkdude commented Jan 16, 2023

@mk I pushed a fix for Windows. I also fixed some reflection issues in analyzer.clj (since I had to do some interop I found it better to enable that). I also added an alias for the cognitect test runner which I find easier to work with, I hope that's ok.

@mk mk merged commit 3988bda into main Jan 19, 2023
@mk mk deleted the fix-hash-warning branch January 19, 2023 08:49
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.

Latest clerk errors on notebook analyses, ->hash must be ifn?
3 participants