-
Notifications
You must be signed in to change notification settings - Fork 0
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
bump to maili #23
bump to maili #23
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.
overall LGTM. why is this needed now? are we sure maili is going to be more stable than op-allow? are op-succint and kailua also going to be using maili? if not we might run into dependency hell issues
bin/client/justfile
Outdated
@@ -69,7 +69,7 @@ run-client-native-against-devnet verbosity='' block_number='' rollup_config_path | |||
L1_BEACON_RPC="http://127.0.0.1:5052" | |||
L2_RPC="http://127.0.0.1:9545" | |||
ROLLUP_NODE_RPC="http://127.0.0.1:7545" | |||
ROLLUP_CONFIG_PATH="{{justfile_directory()}}/../../optimism/.devnet/rollup.json" | |||
ROLLUP_CONFIG_PATH="/home/ubuntu/op-main-repo/.devnet/rollup.json" |
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.
why are you reverting to a path that only works on your machine? did the relative one not work?
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.
nah, just my local thing, will revert this line
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.
fixed
upgrade to maili would work, unless then hokulea won't compile But I am fine to just find a version that works with op-succint or kailua |
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.
Overall LGTM. Haven't looked at rust's package manager enough, but overall feel like we should very rarely specify minor versions as minimum requirements.
serde = { version = "1.0.217", default-features = false } | ||
serde_json = { version = "1.0.135", default-features = false } |
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.
serde = { version = "1.0.217", default-features = false } | |
serde_json = { version = "1.0.135", default-features = false } | |
serde = { version = "1.0", default-features = false } | |
serde_json = { version = "1.0", default-features = false } |
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.
if we don't absolutely need those minimum versions, then don't specify them. makes it easier to get into dependency hell.
Upgrade to maili