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

Remove "ISSL6U" keyword from file name and class name #31

Merged
merged 3 commits into from
Jun 17, 2023

Conversation

suzuki-toshihir0
Copy link
Member

@suzuki-toshihir0 suzuki-toshihir0 commented Jun 14, 2023

概要

SSIA

Issue

詳細

"ISSL" keyword in file names and class names are removed for OSS publication.

検証結果

build in VS2022 was verified

影響範囲

big

補足

何かあれば

注意

  • 関連する Projects が存在する場合,それの紐付けを行うこと
  • Assignees を設定すること
  • 可能ならば Reviewers を設定すること
  • 可能ならば priority ラベルを付けること

@suzuki-toshihir0
Copy link
Member Author

この際にヘッダファイルの拡張子も h から hpp に変えたほうがよさそうですかね?

@200km
Copy link
Member

200km commented Jun 14, 2023

s2e-coreのルールに完全に沿うようにするなら、

  • include guardをifdefに統一する
  • ヘッダをhppにする
  • ディレクトリも含めて全て小文字snake caseにする

などもありますね。
詳細は下記の通りです。

https://github.com/ut-issl/s2e-documents/blob/develop/General/CodingConvention.md

@200km
Copy link
Member

200km commented Jun 14, 2023

別PRでも良いと思いますし、必要なら私も手伝います。

@suzuki-toshihir0
Copy link
Member Author

suzuki-toshihir0 commented Jun 15, 2023

ありがとうございます.ここではISSL6Uキーワードをなくすことを目的にしたいので,PRは分けたいですね...トピックごとにわけて,お手伝いいただけますと嬉しいです.

このPRはマージして,以下の進め方にするのはどうでしょうか?(a, b, cを分担するイメージ)

  1. feature/fix_format ブランチを切る
  2. feature/fix_format に向けたPRを発行する.
    • PRは3個に分ける.
      1. インクルードガードの修正
      2. ヘッダをhppにする修正
      3. snake_caseへの修正

@200km
Copy link
Member

200km commented Jun 16, 2023

別PRで分担ということでOKです。
ただ、a,b,cは確実にコンフリクトするので、二人で分けて同時作業するのは逆に面倒な気はしています。
時間はかかるが一人がシーケンシャルに作業して一つずつマージしていくか、時間短縮のため一気に作業するかかなと思います。後者はレビュワーは大変かもですが、僕がレビュワーの場合は特に気にはしません。

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します。

@suzuki-toshihir0
Copy link
Member Author

確かにコンフリクト考えるとむしろ面倒ですね...私のほうでがっと作業するので,まとめてレビューしていただいてもよいでしょうか?

@suzuki-toshihir0
Copy link
Member Author

とりあえずこれのマージはします.

@suzuki-toshihir0 suzuki-toshihir0 merged commit 39c9ae8 into develop Jun 17, 2023
@200km 200km deleted the feature/rename_issl6u_files branch June 18, 2023 13:08
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.

Rename files which contain ISSL6U keyword
2 participants