Skip to content
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

#101 バージョン0.4.0未満のプロジェクトファイルへの対応 #102

Merged
merged 17 commits into from
Aug 18, 2021

Conversation

Segu-g
Copy link
Member

@Segu-g Segu-g commented Aug 16, 2021

プロジェクトファイルについての操作をstore内のindex.tsからstore内のstore/projectフォルダに移し,store/project/version0.3.0.ts , store/project/version0.4.0.tsのようにプロジェクトファイルの形式を記述する構造にしてみました.
前後のバージョン間のプロジェクトファイルの変換関数(updater)を記述することでマイグレーションを実現しています.
fix #101

@Segu-g Segu-g marked this pull request as draft August 16, 2021 01:07
@Segu-g Segu-g changed the title #101 [WIP] プロジェクトファイルのバージョン間マイグレーション #102 [WIP] プロジェクトファイルのバージョン間マイグレーション Aug 16, 2021
@Segu-g Segu-g changed the title #102 [WIP] プロジェクトファイルのバージョン間マイグレーション #101 [WIP] プロジェクトファイルのバージョン間マイグレーション Aug 16, 2021
@Segu-g Segu-g marked this pull request as ready for review August 17, 2021 03:48
@Segu-g
Copy link
Member Author

Segu-g commented Aug 17, 2021

2021年8月17日現在 mainブランチにおいてソフトウェアのバージョンは0.3.1と設定されており、プロジェクト形式の更新バージョンを0.4.0としています。従って現在のmainブランチから生成されたプロジェクトフォルダは形式が不正となり、自身で読み込むことができません.
バージョン0.3.0 , 0.3.1のプロジェクトファイルは過去のリリースから作成したものを用い、バージョン0.4.0のプロジェクトファイルはmainブランチから作成したプロジェクトファイルに対してテキストエディタ等でappVersionを"0.3.1"から"0.4.0"に書き換えたものを使用してください。

@Segu-g Segu-g changed the title #101 [WIP] プロジェクトファイルのバージョン間マイグレーション #101 プロジェクトファイルのバージョン間マイグレーション Aug 17, 2021
@Hiroshiba
Copy link
Member

ソフトウェアのバージョンをプロジェクトファイルのバージョンにするとそういった煩わしさがあるんですね。なるほどです。
releaseブランチを切ってmainブランチのバージョンは先に上げておくか、プロジェクトファイルバージョンを別に作るか、といったところでしょうか・・・
hotfixが入ったときに強い設計にしておきたいのですが、どうすればいいでしょう。

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

やりたいこと(プロジェクトファイルのマイグレーションを型レベルで保証する)はとてもかっこいいと思うのですが、ライブラリを管理するくらいの気合が必要な実装だと感じました。

このプルリクエストには0.3.1のプロジェクトファイルを0.4.0へマイグレーションする機能と、マイグレーションのインターフェースの機能が備わっていて、前者は急ぎたい一方、後者は議論が必要で時間がかかるかもしれません。
とりあえずマイグレーションするだけ(load時のバージョンが0.4.0未満ならcharactorをcharacterにreplaceする)のコードをマージして、後者はゆっくり実装していくのはどうでしょう。

src/store/project/index.ts Outdated Show resolved Hide resolved
src/store/project/index.ts Outdated Show resolved Hide resolved
src/store/project/index.ts Outdated Show resolved Hide resolved
src/store/project/index.ts Outdated Show resolved Hide resolved
@Segu-g
Copy link
Member Author

Segu-g commented Aug 17, 2021

ひとまず全てのバリデーションチェックを捨て,charactorIndexをcharacterIndexに書き換えるだけのコードをに差し替えました.
マイグレーションのコードはブランチを切ったのでissue等で議論しましょう.

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

issueでの議論、良いと思います。
コードもすごくスッキリして良いと思います。 

_typos.toml Outdated Show resolved Hide resolved
_typos.toml Show resolved Hide resolved
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

先に入った https://github.com/Hiroshiba/voicevox/pull/114 が偶然コンフリクトしているようです。

@Segu-g Segu-g requested a review from Hiroshiba August 17, 2021 20:07
@Segu-g Segu-g changed the title #101 プロジェクトファイルのバージョン間マイグレーション #101 バージョン0.4.0未満のプロジェクトファイルへの対応 Aug 17, 2021
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

remove/addされた行数を比較して気づいたのですが、projectSchemaが消えてValidationがなくなっていそうです

@Segu-g
Copy link
Member Author

Segu-g commented Aug 18, 2021

意図を勘違いしていたようで申し訳ありません。
バージョン0.3.0と0.4.0のSchemaに対してバリデーションチェックを行いながら、0.3.0の時は0.4.0の型へのキャストを書くという事でしょうか?

@Hiroshiba
Copy link
Member

最新版になったものをバリデーションするのが良いのかな、と思っています。

最新版(0.4.0版)をロードした時
json.load → validation

0.3.1版をロードした時
json.laod → migration → validation

という感じです。

@Segu-g Segu-g requested a review from Hiroshiba August 18, 2021 12:39
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

バージョン0.3.1→0.4.0への変更でプロジェクトファイルの読み込みができなくなる
3 participants