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 user cmake option #132

Merged
merged 5 commits into from
Oct 12, 2023
Merged

Conversation

sksat
Copy link
Member

@sksat sksat commented Oct 12, 2023

概要

#86 に引き続き,C2A user 側の CMake option についても整理する

Issue / PR

詳細

各 C2A user で実際にどのような CMake option が定義されているかは未知だが,#86 でコーディング規約を変更したため,コーディング規約に遵守している限りは,最低限 大文字の SNAKE_CASE で,C2A_ prefix に揃えているはず.
この PR では,example user の CMake option を新しい規約に遵守させる.

検証結果

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

影響範囲

  • C2A user 固有の CMake option を指定してビルドしている場合に対して breaking(workflows-c2a など)
  • example user は C2A user のテンプレートとして用いられているため,各 C2A user には基本的にこの PR で変更する option が含まれているはず.この PR はそれらの option についての変更先の名前の標準的な命名を推奨する意味も持つ.

@sksat sksat added enhancement New feature or request priority::high priorityg high tools labels Oct 12, 2023
@sksat sksat self-assigned this Oct 12, 2023
@@ -16,11 +16,11 @@ option(USE_SCI_COM_WINGS "Use SCI_COM_WINGS" ON)
# そちらのバッファが詰まってSILSの動作が止まることがあるので注意すること!
option(USE_SCI_COM_UART "Use SCI_COM_UART" OFF)
Copy link
Member Author

Choose a reason for hiding this comment

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

USE_SCI_COM_WINGSUSE_SCI_COM_UART もどうにかしたい

  • 今後は WINGS は optional な存在となるため,USE_SCI_COM_WINGS は default OFF とする
  • SCI って何?

Copy link
Member Author

Choose a reason for hiding this comment

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

SCI はルネサス語っぽい(Serial Communication Interface)

Copy link
Member

Choose a reason for hiding this comment

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

やるなら
uart_sils_sci_if
ccsds_sils_sci_if
も同時に消したい

Copy link
Member Author

Choose a reason for hiding this comment

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

この PR では一旦 C2A_ prefix だけ付ける

Copy link
Member

Choose a reason for hiding this comment

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

OK

@sksat
Copy link
Member Author

sksat commented Oct 12, 2023

rebase

@sksat sksat force-pushed the feature/refactor-user-cmake-option branch from 50f9edb to 50abcaa Compare October 12, 2023 15:05
Copy link
Member

@meltingrabbit meltingrabbit left a comment

Choose a reason for hiding this comment

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

wfの方の修正がいるので,一旦approveとりけし.内容はOK

@sksat
Copy link
Member Author

sksat commented Oct 12, 2023

workflows-c2a 側の対応は #136 で入る

@sksat
Copy link
Member Author

sksat commented Oct 12, 2023

workflows-c2a の対応入ったので rebase

@sksat
Copy link
Member Author

sksat commented Oct 12, 2023

再度修正: #137

@sksat
Copy link
Member Author

sksat commented Oct 12, 2023

rebase

@sksat sksat force-pushed the feature/refactor-user-cmake-option branch from 836b52c to 1a54a0e Compare October 12, 2023 16:12
@sksat sksat requested a review from meltingrabbit October 12, 2023 16:12
@sksat sksat merged commit 4220e56 into develop Oct 12, 2023
35 checks passed
@sksat sksat deleted the feature/refactor-user-cmake-option branch October 12, 2023 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority::high priorityg high S2E tools WINGS
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants