feat: expose c1 migration cmds #186
Conversation
There was a problem hiding this comment.
LGTM
Should we wait to merge till the cli changes are merged so we can manually test this? ceramicnetwork/rust-ceramic#326
keramik/src/ipfs.md
Outdated
| - ipfs: | ||
| rust: | ||
| migrationCmd: | ||
| - from-ipfs -i /data/ipfs/blocks -o /data/ipfs/db.sqlite3 --network dev-unstable |
There was a problem hiding this comment.
nit, since the cli is not merged yet the api is not stable but this should read:
from-ipfs -i /data/ipfs/blocks -o /data/ipfs/ --network dev-unstable
Yes, that sounds good 👍🏼 |
nathanielc
force-pushed
the
feature/aes-125-keramik-should-expose-a-migration-cmds-setting
branch
2 times, most recently
from
f32b0f9
to
0dfb94f
Compare
|
@smrz2001 I made a new commit (and rebased off main) can you take a look ? |
nathanielc
force-pushed
the
feature/aes-125-keramik-should-expose-a-migration-cmds-setting
branch
from
0dfb94f
to
20285ab
Compare
| command: Some( | ||
| vec!["/usr/bin/ceramic-one", "migrations"] | ||
| .into_iter() | ||
| .chain(cmd.iter().map(String::as_str)) |
There was a problem hiding this comment.
If we moved the chain below the map on the next line, could we take out the as_str mapping?
There was a problem hiding this comment.
No, we do not own the strings in cmd so the as_str converts from &String to &str. We could instead convert directly to String but that still requires a map on the cmd iterator.
There was a problem hiding this comment.
So its about the same either way, I don't have a strong reason for one over the other.
There was a problem hiding this comment.
Ah understood, thanks.
Updates LGTM! |
* feat: expose c1 migration cmds * chore: build fixes * fix: use list of args for migration_cmd * fix: tests --------- Co-authored-by: Nathaniel Cook <[email protected]>
No description provided.