CodeFactor を導入しようと思います

[m-tmatma]

CodeFactor を導入しようと思います

• CodeFactor (公式サイト)
• Marketplace CodeFactor
• CodeFactorがスーパー便利だった

以下は sakura-editor の fork に対して設定したものです。 https://www.codefactor.io/repository/github/m-tmatma/sakura

[berryzplus]

導入に反対します（笑

[m-tmatma]

紹介動画です。これを見て考え直してください。(笑 https://youtu.be/0wL1bgoya2U

[berryzplus]

紹介動画です。これを見て考え直してください。

これがどういうもので、どんな利点があって、導入すると何が起きるのか。 それを自分の言葉で説明してください。URLを貼って「読め」ってのは違う気がします。 たぶん、分かっててやってそうなので「（笑」を付けました。

そしてサクラエディタにとって、このツールは危険なんじゃないかと思っています。

[m-tmatma]

失礼。書きかけが残っていたので送信してないと思って再送信してしました。

[m-tmatma]

そしてサクラエディタにとって、このツールは危険なんじゃないかと思っています。

どういう意味ですか?

これがどういうもので、どんな利点があって、導入すると何が起きるのか。

cppcheck と同様に静的解析ツールです。

GitHub と連携して、解析してくれて PR のタイミングで新たに問題を埋め込んでいないか? あるいは問題を修正したかをチェックしてくれます。

コードの重複や複雑すぎるコードも指摘してくれます。 別に問題を検出していても放置していても大丈夫なので、気が向いたときに治すのでもいいです。

[berryzplus]

コードの重複や複雑すぎるコードも指摘してくれます。

このツールが危険だと思う理由はここです。

静的解析ツールはPRの文脈を理解してくれません。 文脈や経緯を一切無視して、ツール作成者が勝手に決めた価値基準でレビューを行ってくれます。 それは困るんです。文脈や経緯を考慮したレビューに注力したいと思っています。

もしぼくらが全員、どうやってコードレビューしたらいいか分からない人だったら、指摘の仕方を勉強するためにこういうのもありだと思うんです。でもきっと、違いますよね？

別に問題を検出していても放置していても大丈夫なので、気が向いたときに治すのでもいいです。

対応しなくていい、を浸透させるのが結構大変なんじゃないかと思っています。

というか、 m_tmatma さんがこういうツールの指摘を無視して進めるのを「よし」とすると思ってませんでしたｗ

[beru]

導入する事は良いと思いますが実際に問題点が列挙される事と、コードが実際に改善される事との間には距離があると思います。後者については現状人力で行うしかないので。

コードの改善をするには誰かがとにかく作業をしないといけないので、リファクタ計画みたいなのをプロジェクトの目標として用意して誰か奴隷(共)をアサインするか、計画は特に立てないけどとにかくリファクタしたいキチガイ(達)に自発的に取り組んでもらうか、まぁ表現はともかくとしてテストを含めたコード書きとレビューにマンパワーが必要な事は今と変わりがないので、問題点が示されたとしても改善に寄与するかというと結局はやる気やらせる気次第かなと思います。

PRもしくは特定のブランチのコミット単位でチェックするにしても、差分だけでなく全体を対象に問題点を検出する場合（全体では無く差分を対象にするにしても対象の判断に文脈が関係するので人間が満足するくらい精度の良い判断をプログラムが行うのは現状厳しいと思いますが）動かす度にお前のプログラムはまるでそびえたつ :shit: のようだというレポートをデータセンターが地球を緩やかに温暖化しながら出してくれるわけで、「そんなの知ってるよ！」と毎回心の中で突っ込むか、「違うよ。全然違うよ。」とマーク・パンサー氏のように心を偽るか、その間で揺れ動きそうです。

[m-tmatma]

というか、 m_tmatma さんがこういうツールの指摘を無視して進めるのを「よし」とすると思ってませんでしたｗ

理想的には直したいのですが、 現実的には難しいので、ある意味妥協です。

新規のプロジェクトだと全部修正して 0 の状態を維持したいです。

想定としては、 PR を投げたときに、なるべく警告が増えないようにしましょう、 ぐらいを考えています。

偽陽性が多すぎて役にたたないかもしれないし、逆にレビューアーが全然気づかなったのを 検出してくれて、導入してよかった、となるかもしれないです。

とりあえず使ってみたらいいんじゃない? というスタンスです。 全然つかえないじゃんとなれば、GitHub との連携を切ればいいだけです。

一応、偽陽性の場合は、false negative というのを設定すれば警告を消せますし、 Won't Fix というのも選べました。

対応しなくていい、を浸透させるのが結構大変なんじゃないかと思っています。

appveyor で cppcheck を commit の毎に実行していますが、 基本的に結果は無視の状態です。既に浸透しているのかも(笑)

[berryzplus]

偽陽性が多すぎて役にたたないかもしれないし、逆にレビューアーが全然気づかなったのを 検出してくれて、導入してよかった、となるかもしれないです。

問題意識がうまく伝わってないように思います。

以下は sakura-editor の fork に対して設定したものです。 https://www.codefactor.io/repository/github/m-tmatma/sakura

これ、一応ちゃんと見てるんですけど、指摘内容は適切だと思います。 偽陽性（＝ツールの精度）に関しては心配ないと考えています。 つまり、ツールで検出された問題は対処すべきです。（「偽」じゃないっすｗ

対処すべき問題と分かったうえで置いておくこともあるんです。 検出された問題がそんなに深刻じゃないから置いといてもいい、ではなく。 偽陽性だから対処しなくていい、というわけでもなく・・・。 やるべきか？やらざるべきか？と考えたら「やるべき」なんです。 でも、他との兼ね合いとか人的リソースとかを考えて「放置」を選択するんです。

ツールを入れるとコード上の問題点が自動で列挙されるようになります。 PRのたびに、PRとは関係ない既存コードの問題点を見ることになります・・・。

PRは、なんらかの目的で既存コードの一部を変えるように出すものです。 「目的と関係ない部分」には通常手を入れません。

しかし、ツールはそんなこと関係なくマズいコードに指摘を出すわけです。 PRの目的や趣旨の評価に集中できなくなるんじゃないか？というのが懸念です。

---

導入とは別の話

ツール自体に関してのぼくの評価は高めです。 リファクタリングの勉強をする教材としてはいいんじゃないかと思います。

リファクタリング技術は座学で覚えるものじゃないです。 実際に手を動かす実践なくして身に付くものじゃないと思います。 このツールで指摘された部分を片っ端から直してみる、とか そういう個人的な試みの補助としては良さそうに思います。

[berryzplus]

appveyor で cppcheck を commit の毎に実行していますが、 基本的に結果は無視の状態です。既に浸透しているのかも(笑)

ええっ！そうなの？ｗ　＞　既に浸透

単に結果が見辛くて見る気にならないだけのような。 （見るのしんどいからみたくても見れないっていう・・・

cppcheck の警告もいつかは対応したいですが、PRのたびに確認する使い方は考えていません。 codefactorの場合、「PRのたびに確認」という運用が、現実的にできそうです。 手軽に確認できるようになったとき、熱くスルーすることができるか否か、という話です。（熱さ関係ねぇ・・・

[m-tmatma]

静的解析ツールはPRの文脈を理解してくれません。 文脈や経緯を一切無視して、ツール作成者が勝手に決めた価値基準でレビューを行ってくれます。 それは困るんです。文脈や経緯を考慮したレビューに注力したいと思っています。

↓ は ↑ に対する指摘です。

偽陽性が多すぎて役にたたないかもしれないし、逆にレビューアーが全然気づかなったのを 検出してくれて、導入してよかった、となるかもしれないです。

問題意識がうまく伝わってないように思います。 これ、一応ちゃんと見てるんですけど、指摘内容は適切だと思います。

↑ これは、結局どっちやねん、って気がします。

偽陽性（＝ツールの精度）に関しては心配ないと考えています。

偽陽性はあります。

自分で試していたときにライセンス文書の中でセミコロンで終わる箇所をコメントアウトしたコードと認識して コメントアウトしたコードがあるよ みたいな警告を出してました。

それを手動で 'false negative' と設定しました。 (過去に false negative に設定した警告を後で確認する方法に関してはわからなかったです)

ツールを入れるとコード上の問題点が自動で列挙されるようになります。 PRのたびに、PRとは関係ない既存コードの問題点を見ることになります・・・。

別のプロジェクトで試しましたが、 PR で指摘するのは、問題が解消したか、あるいは新たに発生したかだけの認識です。

以下で 2 件の問題を修正しています。 https://www.codefactor.io/repository/github/m-tmatma/matchevaluatortest/pull/fixed/4

問題としてはプロジェクト全体としては以下のように残っています。 https://www.codefactor.io/repository/github/m-tmatma/matchevaluatortest/issues

しかし、PR としては "New Issues" としては、No issues found となっています。 https://www.codefactor.io/repository/github/m-tmatma/matchevaluatortest/pull/4

[m-tmatma]

単に結果が見辛くて見る気にならないだけのような。 （見るのしんどいからみたくても見れないっていう・・・

cppcheck の結果を見るのがしんどいから、見やすくしましょう、という問題提起もないので 実質無視してます。

[ds14050]

みなさん(特に @berryzplus さん)は深刻に考えすぎなんじゃないでしょうか。

自分は機械の意見も聞いてみたいし、もっともだと思えば従う、役に立たなくても賑やかしにはなるだろうくらいのスタンスで、反対する理由がありません。

懸念といえばビルド時間で、cppcheck や doxygen、文字コードチェックといった１度だけ実行すればいいジョブは AppVeyor のビルドマトリクスから除外して別の CI に移したいなと、それができてから導入されるといいな、という感じです。

[berryzplus]

自分は機械の意見も聞いてみたいし、もっともだと思えば従う、役に立たなくても賑やかしにはなるだろうくらいのスタンスで、反対する理由がありません。

懸念といえばビルド時間で、cppcheck や doxygen、文字コードチェックといった１度だけ実行すればいいジョブは AppVeyor のビルドマトリクスから除外して別の CI に移したいなと、それができてから導入されるといいな、という感じです。

こんな感じの扱いでよければ、導入してもよいと思います。

機械を無視するのは簡単ですが、人間を無視するのはキッツいです。最大の懸念はそこにあるので。

今やってるバッチ改善が落ち着いた頃に、とりあえず入れてみる、でどうでしょう。やってみないと実時間は分からんので。

[m-tmatma]

appveyor から実行するわけではないので 実行時間は全く増えないです。

また同様の別のサービスを利用して cppcheck も appeyor から実行しなくて いいようにできるのを検討中です。

[m-tmatma]

↑ 必要なことは、このサービスを使えるように権限を与えて、 markdown にリンクなり、バッチを貼るだけです。

[berryzplus]

↑ 必要なことは、このサービスを使えるように権限を与えて、 markdown にリンクなり、バッチを貼るだけです。

じゃあGOで :smile: バッチのほうがカッコいい気がするからバッチがいいな～。

で、いいですよね？　> @KENCHjp さん

[KENCHjp]

で、いいですよね？　> @KENCHjp さん

問題ないっす！

[m-tmatma]

https://github.com/sakura-editor/sakura/pull/693 で badge 貼りました。

[m-tmatma]

PR ごとの issue 一覧は https://www.codefactor.io/repository/github/sakura-editor/sakura/pulls で確認できる。

[berryzplus]

やっぱり issue 増えてるPRは「なんだ？」って見ちゃいますね。

ぼくが出して塩漬けされてるPRで new issues +3 がありました...orz https://www.codefactor.io/repository/github/sakura-editor/sakura/pull/493

これは速度確保のためにしょうがない複雑さなんですけど、実際見ると凹みますね・・・。