-
Notifications
You must be signed in to change notification settings - Fork 60
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
feat: removed external dependencies from miden-objects #1186
Conversation
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.
Looks good to me! I think we can add a couple of re-exports and then this is ready.
prettier::PrettyPrint, | ||
Program, | ||
}; | ||
use assembly::Compile; |
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.
We can add a re-export of Compile
in lib.rs and then import it from crate
as well.
Program, | ||
}; | ||
use assembly::Compile; | ||
use vm_core::prettier::PrettyPrint; |
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.
I think we can also add a re-export of PrettyPrint
.
AdviceMap, Program, | ||
}; | ||
use vm_processor::{AdviceInputs, DeserializationError}; | ||
use vm_core::mast::{MastForest, MastNodeId}; |
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.
Let's also add a re-export of MastForest
and MastNodeId
.
use super::{ | ||
Asset, ByteReader, ByteWriter, Deserializable, DeserializationError, Digest, Felt, Hasher, | ||
NoteError, Serializable, Word, WORD_SIZE, ZERO, | ||
use super::Asset; |
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.
Can you replace this Asset
import with an import from crate
? Thanks!
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.
Looks good to me!
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.
Looks good! Thank you!
@varun-doshi - could you fix the lints? After this, we should be good to merge. |
36a59e9
to
ebcd159
Compare
Ref #1145
removed external dependencies from
miden-objects