OpenSiv3D icon indicating copy to clipboard operation
OpenSiv3D copied to clipboard

Pull Requestに対して自動でlinterを実行する

Open polyester-CTRL opened this issue 3 years ago • 17 comments

追加する機能の内容 | Describe the solution you'd like

  • GitHub Actionsを使って、OpenSiv3D本体のリポジトリにPull Requestを出した際に自動でlinterを実行したいです。
  • 具体例としては、 インデントがtabに統一されていないコードなどがあったときに、レビュアーが指摘する前にエラーや警告を出します。

その機能の追加によって解決する問題 | Is your feature request related to a problem? Please describe.

  • レビュアーの負担を軽減することが期待できます。

備考 | Additional context

polyester-CTRL avatar Jun 20 '22 12:06 polyester-CTRL

ご提案ありがとうございます。検討します。 具体的な追加コードは用意できますか? 実装会での作業とするのも良いです。

Reputeless avatar Jun 20 '22 22:06 Reputeless

追加コードについてはこれから用意します。 もし日程的に実装会に参加できるようなら、そこで作業しようと思います。

polyester-CTRL avatar Jun 21 '22 05:06 polyester-CTRL

ありがとうございます。心強いです。

Reputeless avatar Jun 21 '22 05:06 Reputeless

コーディング規約を細かく設定できる点から、clang-formatを使用する予定です。 もし別のツールが良いなどの意見がありましたら、コメントをお願いします。

polyester-CTRL avatar Jun 24 '22 15:06 polyester-CTRL

メジャーなツールなので大丈夫だと思います。 設定するのはタブスペース、{ } 位置ぐらいでいいのかなとは思っています。

Reputeless avatar Jun 25 '22 03:06 Reputeless

clang-formatを試しに使ってみたところ、一部のルールだけチェックするようなことはできないようです。 例えば、ポインタや参照の位置( int* a; もしくは int *a; )の違い、#include <foo># include <foo>となっていることなどもすべて警告として表示されることが分かりました。 できるだけ既存のコードで警告を出さないようにしたいのですが、警告を出さないようにオプションを調整するところで手間取っています。

polyester-CTRL avatar Jul 10 '22 16:07 polyester-CTRL

v0.8 世代で 0 からフルスクラッチで Siv3D を書き直すので、そのときに導入するのも手かもしれないなと思いました。

Reputeless avatar Jul 11 '22 03:07 Reputeless

0から書き直すタイミングなら問題なく導入できそうですね。 導入する際は、.clang-formatでコーディング規約を設定していただけるとスムーズにlinterを設定できると思います。よろしくお願いします。

polyester-CTRL avatar Jul 11 '22 07:07 polyester-CTRL

参考までに、現在設定している.clang-formatオプションです。完全に同じではありませんが、今のSiv3Dのソースコードに近い記法にフォーマットできます。

Language: Cpp
BasedOnStyle: LLVM
Standard: c++20
UseTab: ForIndentation
NamespaceIndentation: All
TabWidth: 1
IndentWidth: 1
PointerAlignment: Left
BreakBeforeBraces: Allman
AllowShortIfStatementsOnASingleLine: false
IndentCaseLabels: false
ColumnLimit: 0
AccessModifierOffset: -1
FixNamespaceComments: false
BraceWrapping:
  AfterFunction: true
  BeforeElse: true
  AfterNamespace: true
  AfterStruct: true

polyester-CTRL avatar Jul 11 '22 07:07 polyester-CTRL

前向きに導入を検討します! ちなみに # include ← この空白は Siv3D の個性なのでできれば残したいのですが、 警告回避方法が無いのであれば Siv3D 側のルールの見直しも必要になるのかな・・・

Reputeless avatar Jul 11 '22 07:07 Reputeless

# includeは空白を残すオプションを探しているのですがまだ見つけられていません・・・ 他にも、1行に複数の文があるコードは読みやすいとしても許容されないようです。 https://github.com/Siv3D/OpenSiv3D/blob/03667011e99263c6609f19c83800e5f6627cc4cc/Siv3D/src/Siv3D/AnimatedGIFReader/AnimatedGIFReaderDetail.cpp#L30

polyester-CTRL avatar Jul 11 '22 15:07 polyester-CTRL

別のツールについても調査したところ、editorconfig-checker というツールのほうが向いていることが分かりました。 何が良いかというと、設定した一部のルールだけをチェックしてくれるので、既存のコードでエラーが出にくい点です。

チェックできるのは以下の6つです。

  • end_of_line: 改行文字をlf, crlf, crのいずれかに統一
  • insert_final_newline:ファイルの末尾を改行で終わらせる
  • trim_trailing_whitespace:行末から空白を削除
  • indent_style:インデントをタブまたはスペースに統一
  • indent_size:インデントは何文字分かを設定
  • max_line_length:1行あたりの文字数を制限

clang-formatとは異なり、一部のルールを無視することも可能です。 やや機能は少なめですが、最低限の自動的な確認には役立つと思います。

polyester-CTRL avatar Jul 18 '22 06:07 polyester-CTRL

来月あたりから、v0.8 実験リポジトリを始めます。 そこでぜひ運用テストをしましょう。 https://github.com/Siv3D/siv8

Reputeless avatar Jul 18 '22 08:07 Reputeless

分かりました。よろしくお願いします。

polyester-CTRL avatar Jul 18 '22 16:07 polyester-CTRL

もし良かったら、@polyester-CTRL さんのリポジトリで簡単なプロトタイプを作って、運用例を見せてもらえると導入がスムーズです。

Reputeless avatar Jul 23 '22 23:07 Reputeless

https://github.com/polyester-CTRL/ci-practice 作りました。mainブランチにプルリクエストを出す、またはプッシュするとlinterが走ります。

polyester-CTRL avatar Aug 01 '22 17:08 polyester-CTRL

ありがとうございます。 v0.8 整備時に使ってみます。

Reputeless avatar Aug 02 '22 17:08 Reputeless