Skip to content
This repository has been archived by the owner on Apr 24, 2023. It is now read-only.

Remove metatransaction.core import that causes problem with mount. #1691

Merged
merged 3 commits into from
Sep 30, 2020

Conversation

scrosby
Copy link
Member

@scrosby scrosby commented Sep 29, 2020

Changes proposed in this PR

  • Remove metatransaction.core import that causes problem with mount.

Why are we making these changes?

Avoids an incomprehensible error with mount. Diagnoses #1370 with a workaround.

# Conflicts:
#	scheduler/src/cook/quota.clj
@scrosby scrosby requested a review from DaoWen September 29, 2020 23:20
Comment on lines 342 to 343
; If you get an error about "Can't embed object in code, maybe print-dup not defined: clojure.lang.Delay"
; The issue is about metatransaction.core seems to be incompatible with mount. It cannot be in the dependency tree.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment seems overly specific in blaming metatransaction.core for the error. What if we have another namespace that causes the same error when require'd into the dependency tree? How did you trace the blame back to metatransaction.core? The first sentence on 343 has a typo, so it needs to be reworded anyway.

Maybe just mention the error and link to issue #1370 instead. I probably should have done something like that when I opened the issue.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was able to create the error only adding a dependency to metatransaction.core, and was able to make it go away by deleting the reference. The issue you found adding progress was because of this dependency chain: cook.progress -> cook.tools -> metatransaction.core. I reworded it.

@DaoWen DaoWen merged commit 7ac1171 into twosigma:master Sep 30, 2020
@scrosby scrosby deleted the outgoing/fix-uberjarbuild branch October 9, 2020 16:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants