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

トップディレクトリのファイルを整理する #740

Closed
m-tmatma opened this issue Jan 1, 2019 · 10 comments
Closed

トップディレクトリのファイルを整理する #740

m-tmatma opened this issue Jan 1, 2019 · 10 comments

Comments

@m-tmatma
Copy link
Member

m-tmatma commented Jan 1, 2019

トップディレクトリのファイル数が多くなってきたので、整理する

#826 (comment)

もっと理想を言えば、appveyor と共通で利用するバッチ類を ci ディレクトリに、appveyor のものを ci/appveyor ディレクトリに

まあ、ぼちぼちと考えます。

@ghost
Copy link

ghost commented May 4, 2021

一旦ここにもまとめておきます。次の通り移動・編集を行いました。

このほか、以下に示すファイル移動を予定しています

  • addDoxygenFileComment.bat → 廃止
    • 内容が python addDoxygenFileComment.py %~dp0/sakura_core だけで、必要性を感じませんでした。
  • addDoxygenFileComment.md → tools/addDoxygenFileComment.md
  • addDoxygenFileComment.py → tools/addDoxygenFileComment.py
  • create-big-file.md → tools/create-big-file.md
  • create-big-file.py → tools/create-big-file.py
  • remove-redundant-blank-lines.md → tools/remoe-redundant-blank-lines.md
  • remove-redundant-blank-lines.py → tools/remoe-redundant-blank-lines.py

@berryzplus
Copy link
Contributor

このほか、以下に示すファイル移動を予定しています

  • addDoxygenFileComment.bat → 廃止

    • 内容が python addDoxygenFileComment.py %~dp0/sakura_core だけで、必要性を感じませんでした。
  • addDoxygenFileComment.md → tools/addDoxygenFileComment.md

  • addDoxygenFileComment.py → tools/addDoxygenFileComment.py

  • create-big-file.md → tools/create-big-file.md

  • create-big-file.py → tools/create-big-file.py

  • remove-redundant-blank-lines.md → tools/remoe-redundant-blank-lines.md

  • remove-redundant-blank-lines.py → tools/remoe-redundant-blank-lines.py

ツール系のpythonスクリプトをtoolsフォルダに入れるのは良いと思います。

tools配下に各ツールごとのフォルダを切るか切らないかを悩んでいました。

あと、pyファイルを移動するときに出る SonarCloud 警告(Bugsレベル)をどうするか。

  • ファイルを触るなら修正するのが「筋」です。
  • ファイルを修正するならテストを用意するのが「筋」です。

そんなことやってると配置換えできない気がするので、どこかで妥協するんだろうな、と思っていますが。

@ghost
Copy link

ghost commented May 5, 2021

tools配下に各ツールごとのフォルダを切るか切らないかを悩んでいました。
あと、pyファイルを移動するときに出る SonarCloud 警告(Bugsレベル)をどうするか。

今のところフォルダは切らないつもりでいます。

検証用リポジトリを作成して SonarScan を実行してみましたが、Bugs タイプ の issue は検出されませんでした。
Code Smell タイプのものは大半が「関数・変数の名称が コーディング規約 に違反している」ことへの指摘でした。
https://github.com/kazasaku/sakura-local/pull/2#issuecomment-832430002

@berryzplus
Copy link
Contributor

検証用リポジトリを作成して SonarScan を実行してみましたが、Bugs タイプ の issue は検出されませんでした。

すみません、勘違いだったようです。
https://sonarcloud.io/project/issues?id=sakura-editor_sakura&languages=py&resolved=false&types=CODE_SMELL

CodeSmells129件なので、CやHTMLと比べると全然たいしたことないですが、どっかで対応しないといけない気はします。

@ghost
Copy link

ghost commented May 31, 2021

過去のやり取りを見ていくうち、 remove-redundant-blank-lines は CodeFactor の無効化した警告に対応するために用意されたものであることがわかりました。

  1. 冗長な空行を削除 #719 空行の削除対応を実施(この時に空行を削除するスクリプトを作成)。
  2. [WIP] 冗長な空行を削除 Part2  #724 前回対応時に実施しなかったものを対応しようとした(→可読性を悪化させる変更だったため同意を得られず)。
  3. }; の前の行の空行を削除する #728 閉じ波括弧直前の空行も対象に加え、実際に除去した。
  4. cpplint.py の whitespace/blank_line をチェック対象から外す #959 CodeFactor による空白・空行の警告を無効化した。

この経緯から、このスクリプトはもはや不要と言えるような気がします。

@berryzplus
Copy link
Contributor

個人的には、ルートにあるpyファイル群はまとめてtools/配下に移動したらよいと思っています。

  • tools/
    • ツール名/
      • ツール名.py
      • ツール名.md
      • ツール名-test.py

というのを一律でやりたいです。(個人的に。

@berryzplus
Copy link
Contributor

しまった。ツール名.mdじゃなくてreadme.mdのほうが都合が良さそうっす。

@ghost
Copy link

ghost commented Jun 1, 2021

改めて見たら、途中から会話が全然噛み合っていないように見えてショックでした。

現時点での考え

  1. 無くても良いスクリプト・バッチは削除して、保守対象を削減したいです。
  2. 移動は対象のCodeSmell対応が終わってから一括で行います。
  3. フォルダ分けはファイル移動PRにて階層と名称を例示した上で改めて確認を取ります。

addDoxygenFileComment.batと、remove-redundant-blank-linesはこの第1項より削除したいものとしてカテゴライズされています。

@berryzplus
Copy link
Contributor

ふむ。考え方が違いますね。

要らないスクリプトの削除はできないんじゃないかと思います。
「要る・要らない」の判断基準は人それぞれで、普通に考えて削除提案は独りよがりに見える気がします。
たとえばぼくは「スクリプトに起動バッチなんて不要」派ですが、スクリプトの構造によっては起動バッチがないと不便な場合もあると思います。起動バッチがないと不便なのはスクリプトの構造がマズいと思うんですが、それは単にぼくの主観です。
 

CodeSmells対策も現実的に難しいと思います。
考えたことを考えたまま形にできるのがスクリプトのいいところだと思います。
考えたことを考えたままってのは、つまり「うんこ」です。
スクリプトは本質的に「うんこ」で構わないと思います。
コードを触るなら可能な限りテストを書きたいと思っていますが、
テストを書きたい理由は修正後コードを「うんこ」にしないためです。
やらなくていいことをやろうとしてるんです。
「必要ですか?」と聞かれたら「いいえ」と答えざるをえないことをやろうとしてるんです。
客観的に見て、難しいと思います。

フォルダ分けを移動PRで個別に判断する方式も難しいと思います。
ただでさえPRに対するレビューが付かない状況にあるのに
レビューで「やらないといけない」の判断が1個増えるのはマイナスなのかな、と。

あくまでぼくの主観ですけどね。

@ghost
Copy link

ghost commented Jun 2, 2021

これ以上作業を続行できないと判断し廃案(=Close)とします。

This issue was closed.
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

No branches or pull requests

2 participants