-
Notifications
You must be signed in to change notification settings - Fork 119
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
change: manifest_version
をvvm_format_version
に
#794
change: manifest_version
をvvm_format_version
に
#794
Conversation
個人的にはenumでもいいかな~って思います。 |
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.
LGTM!!
実装ありがとうございます!! 🙇
vvm_format_version
の認識ですが、「VVMのmanifest.jsonやバイナリフォーマットが変わり、今までのコアが正しくVVMを扱えなくなったらインクリメントする」という認識です!
(このあたり)
1箇所コメントしてますが、変えても変えなくても問題なさそう!
変えても変えていなくてもマージしちゃっていただければ!
@sevenc-nanashi 個人的には、そのメッセージが表示されたらVVMの破損を疑う気がします。 |
バージョンはなんとなく文字列にしたのですが、Cargo.lock見たら整数値だったし整数値の方がいいのかも? |
確かにです!! 追記:そこさえ変えればマージしていただいても大丈夫そう! |
整数にしました。 |
内容
#581 (comment)の1.です。
manifest_version
(SemVer)をvvm_format_version
(整数値を文字列化したもの)にします。実装は#581 (comment)で書いたやつの冗長な方を持って来ました。
(エラーメッセージにこだわらないのなら
enum
のやつでもいいかもしれません)関連 Issue
ref #581
その他