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 include directives causing cyclic header file dependencies #44

Closed
wants to merge 1 commit into from

Conversation

kuron99
Copy link
Contributor

@kuron99 kuron99 commented Oct 16, 2024

yakushimaのheaderファイルどうしが #include で相互参照しているものがあり、Ubuntu 24上でclang-tidyからmisc-header-include-cycleとして警告されるので削除したいと思います。
(CIのclang-tidyはUbuntu 22上で実行されているので未検出)

kvs.hの関数宣言がreadability-redundant-declarationとして警告される(NOLINTで回避している)という問題があり、headerのcyclicな状況がこれを引き起こしているのかと思って本修正をしましたが、この変更後でもやはり状況は変わっていないので、そちらは別の問題のようです。ただ、cycleを除去するの自体は意味があると思うので本PRを作成しました。

その他にもyakushimaのヘッダファイル周辺には問題がありそうですが、本PRの範囲外とします

  • Ubuntu 24上のclang-tidyはmisc-header-include-cycle以外にも多数の警告が出されているが、それについては現状のまま
  • kvs.hの各関数はstaticである必要がなさそうに見える
  • そもそもyakushimaはheader onlyである必要もなさそうで、そうであればkvs.hをpublic headerとして他のヘッダファイルと分離できそう

@kuron99 kuron99 requested a review from ban-nobuhiro October 16, 2024 05:55
@kuron99
Copy link
Contributor Author

kuron99 commented Oct 17, 2024

cyclic dependencyを避けるにあたってheader file間のdependencyを取り除きますが、その際にどちらの参照を残すかの変更が恣意的な部分が多く、ポリシーを先に決めるべきという話になりました。本PRはマージせずにcloseします。

@kuron99 kuron99 closed this Oct 17, 2024
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.

1 participant