Upgrade git-stein to v0.5.0 #141
Conversation
| @@ -25,46 +25,46 @@ | |||
| public FinerGitRewriter(final FinerGitConfig config) { | |||
| this.config = config; | |||
| this.builder = new FinerJavaFileBuilder(config); | |||
| setConcurrent(config.isParallel()); | |||
| setPathSensitive(true); | |||
| this.nthreads = config.isParallel() ? 0 : 1; | |||
There was a problem hiding this comment.
nthreadsは0か1を取る変数ですか？もしそうならboolean型の方がよくないでしょうか？
There was a problem hiding this comment.
nthreadsは並列スレッド数で、1だと直列、2以上だとその数、0だとプロセッサ数−1を使う、という仕様になっています。FinerGitはスレッド数をパラメータとして受け取らないので、このようになっています。受け取るように変える手はあります。（これはわかりづらく、stein側のAPIを変えた方が良いかもしれません。良い案あれば欲しいです。）
There was a problem hiding this comment.
FinerGitではparalellかどうかを指定するオプションがありますが，現状のオプションparalellを止めて，使用するスレッド数を直接指定するオプションに変更するのはどうでしょうか？
そのオプションが指定されなかった場合は，プロセッサ数-1を使うで良いと思います．（stein側でプロセッサ数が1の場合は考慮されていますか？）
steinも同じようにしてはどうでしょうか？ 「0だとプロセッサ数-1を使う」は利用者からは直感的ではないですよね．
There was a problem hiding this comment.
ご指摘の通りです。stein では、-p4 (--paralell=4) と指定したら4に、-pと指定したら0になるような仕様になっています。
There was a problem hiding this comment.
FinerGitConfig 変更しても大丈夫ですか？
There was a problem hiding this comment.
args4j では，上記のような，数字を取るが数字の省略もできる引数，というのは作れないような気がします（確認してもらえるとうれしいです）．別オプションにしましょうか．例えば，-p --nthreads=3みたいな．
There was a problem hiding this comment.
FinerGitでは，オプションを指定しない場合は「CPU数-1」のスレッド数で動き，オプションで数字が指定された場合にはその数のスレッドで動作する．
オプションのみ（数字なし）のような入力は受け付けない，でどうでしょうか？
こちらでFinerGitConfig書き換えてもいいんですが，Conflict起きますよね...
| return builder.getFinerJavaModules(base, source); | ||
| protected List<FinerJavaModule> extractFinerModules(final Entry entry, final Context c) { | ||
| final String base = entry.directory + "/" + entry.name; | ||
| final String body = new String(source.readBlob(entry.id, c), StandardCharsets.UTF_8); |
There was a problem hiding this comment.
パスの区切り文字が"/"決め打ちになっていますが，ここってjava.nio.Pathを使った方が安全じゃないでしょうか？
There was a problem hiding this comment.
このパスはGitリポジトリ中のファイルの位置を示すもので、実行環境のOSにおけるファイルシステムに依存していません（チェックアウトしてファイルシステム上に展開することなく扱いますので）。Gitにおいては"/"が唯一のパス区切り文字で、むしろ"\"をファイル名に含められたりします。よって、本来は、ファイルシステム依存性を気にする必要はないはずです。
ただし、このbaseは後にすぐPathに入れられてFinerJavaFileBuilderに渡ります。Pathの機能でディレクトリ名やファイル名を取り出したりするためです。
よって、結局はPathになるので、この時点でPathにしてしまっても差し支えはありません。その場合、FinerJavaFileBuilder#getFinerJavaModulesがPathを受けるようにするべきですか？
There was a problem hiding this comment.
Windows環境でもちゃんと動くのであれば，このままで結構です．
|
コメント返しました。 |
|
コメントしました． |
|
更新しました． |
No description provided.