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

Refactor v4 IfWrapper migration script #180

Merged
merged 2 commits into from
Nov 6, 2023
Merged

Conversation

sksat
Copy link
Member

@sksat sksat commented Nov 2, 2023

概要

v4 migration script のリファクタ

詳細

  • とりあえず IfWrapper rename だけ
  • rename から除外したい対象が微妙に多いので,一番多く find する部分を関数に切り出した
  • */src_core/*, *.xlsm をren

検証結果

test へのリンクや,検証結果へのリンク

影響範囲

v4 migration script

@sksat sksat added priority::high priorityg high tools labels Nov 2, 2023
@sksat sksat self-assigned this Nov 2, 2023
@ToshiAki64
Copy link
Collaborator

@sksat
変更後のスクリプトを実行してみましたが、以下のメッセージがでました。

sed: 入力ファイルがありません

あと、CMakeLists.txt はCommonのままでしたが、意図的ですか?

@sksat
Copy link
Member Author

sksat commented Nov 2, 2023

おや,それだとミスってそうですね

@sksat
Copy link
Member Author

sksat commented Nov 2, 2023

アッ,copilot に変な補完されてた

@sksat
Copy link
Member Author

sksat commented Nov 2, 2023

@ToshiAki64 修正しました.あと,ログ出力を追加しました.

@ToshiAki64
Copy link
Collaborator

ToshiAki64 commented Nov 2, 2023

Common -> common の変換はよさそう。
一方で、Sils -> sils の変換が行われなくなっているような気がしました。

@sksat
Copy link
Member Author

sksat commented Nov 2, 2023

IfWrapper/Sils 以外での参照はあんまり無いかなと思って変えてしまっていましたが,よく考えたらそんなこともないですね.修正します.

@ToshiAki64
Copy link
Collaborator

Sils -> silsもよさそう。

@sksat
Copy link
Member Author

sksat commented Nov 2, 2023

@ToshiAki64 よさそうであれば approve 欲しいです

@sksat sksat changed the title Refactor v4 migration script Refactor v4 IfWrapper migration script Nov 6, 2023
@sksat
Copy link
Member Author

sksat commented Nov 6, 2023

@meltingrabbit こちらこの変更で検証済みなのでこのままマージしてしまいます.別 PR で他の migration script にも同様の修正をします.

@sksat sksat merged commit e8c9403 into develop Nov 6, 2023
35 checks passed
@sksat sksat deleted the hotfix/v4-migration-script branch November 6, 2023 07:14
@meltingrabbit
Copy link
Member

OK

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

Successfully merging this pull request may close these issues.

None yet

3 participants