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

cmd db の Cmd_ prefix を削除 #216

Merged
merged 2 commits into from
Sep 23, 2023
Merged

Conversation

chutaro
Copy link
Contributor

@chutaro chutaro commented Sep 11, 2023

Issue

詳細

issueの通り

検証結果

ビルドチェック (どちらもチェック)

  • SILSでのビルドチェックに通った(CIで確認)
  • vMicroでのビルドチェックに通った

動作確認チェック (いずれかをチェック)

  • SILSでアルゴリズムが想定通りに動いた
  • 実機でアルゴリズムが想定通りに動いた
  • (テレコマ試験の場合)コマンドファイルを使った試験をパスした

試験結果詳細記述場所 or 詳細ログ保存場所へのリンク

N/A

補足

以下と一緒にマージすること

@chutaro chutaro added ✈️ priority::medium priority medium 🐟 patch Patch Update labels Sep 11, 2023
@chutaro chutaro self-assigned this Sep 11, 2023
@chutaro chutaro requested review from sksat and a team as code owners September 11, 2023 12:01
@chutaro chutaro requested review from 200km, suzuki-toshihir0, t-hosonuma and conjikidow and removed request for a team and sksat September 11, 2023 12:01
@200km
Copy link
Member

200km commented Sep 11, 2023

@chutaro 修正ありがとうございます。この修正適応後は、opsファイル内のCmd_も全て消した方が良いということになりますかね?
必要ならAOCS側で別PRで対応しようと思います。

@chutaro
Copy link
Contributor Author

chutaro commented Sep 11, 2023

opsファイル内のCmd_も全て消した方が良いということになりますかね?必要ならAOCS側で別PRで対応しようと思います。

はい、そうなります。c2a-aobc 内にも ops あるんですね、失念していました。ありがとうございます

@200km
Copy link
Member

200km commented Sep 11, 2023

わかりました。一応、ここに大量のopsファイルがあって、全てのコマンドを網羅しようとしているところです。

pytestでもCmd_を削除する必要がありますかね?

@chutaro
Copy link
Contributor Author

chutaro commented Sep 11, 2023

pytest では command_definition.h の enum を参照していて、そこは変更入らないので、修正しなくて大丈夫です

@200km
Copy link
Member

200km commented Sep 11, 2023

そういうことか。そこは間違えてしまいそうですね。pytestコードにコメントとかで注意を書いておこうと思います。

あとは、S2E-CORE側のこの修正もこれに合わせて、修正しないといけないか。これはやるだけなのでやっておきます。

@chutaro
Copy link
Contributor Author

chutaro commented Sep 11, 2023

S2E-CORE側のこの修正

そうですね、よろしくお願いします!

@200km 200km added 🚀 priority::high priority high 🐳 major update Major update and removed 🐟 patch Patch Update 🚀 priority::high priority high labels Sep 13, 2023
@200km
Copy link
Member

200km commented Sep 13, 2023

@chutaro 古いコマンドファイルが使えなくなるということで、後方互換性がなくなるので、patchではなくmajor updateにラベルを変更しました。

Copy link
Member

@200km 200km left a comment

Choose a reason for hiding this comment

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

Approveしますが、major updateなのでマージはAOCSのタイミングでやらせてもらいたいです。

@chutaro
Copy link
Contributor Author

chutaro commented Sep 13, 2023

了解です、よろしくお願いします

@200km
Copy link
Member

200km commented Sep 22, 2023

@chutaro こちら、方針が決まったのでマージ可能です。CDHの良いタイミングでマージお願いします。

@200km 200km mentioned this pull request Sep 22, 2023
3 tasks
@200km 200km added this to the v8.0.0 Major update milestone Sep 22, 2023
@chutaro
Copy link
Contributor Author

chutaro commented Sep 23, 2023

ありがとうございます、マージします

@chutaro chutaro merged commit 5c70de9 into develop Sep 23, 2023
7 of 9 checks passed
@chutaro chutaro deleted the feature/delete_cmd_db_prefix branch September 23, 2023 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants