SoftwareFoundationGroupAtKyotoU / automata

Other
3 stars 10 forks source link

Abolish 7z #322

Closed skymountain closed 9 years ago

skymountain commented 9 years ago

For #254.

krtx commented 9 years ago

zip/unzip.rb のファイル名はクラスが File なので慣習的には zip/file.rb になりますか?(よく分かってない)

chiro commented 9 years ago

既存のモジュールの拡張なので、 *_extension.rb の形の名前になってると嬉しいです。

skymountain commented 9 years ago

zip/unzip.rb のファイル名はクラスが File なので慣習的には zip/file.rb になりますか?(よく分かってない)

長くてもいいなら,慣習的には zip/file/unzip.rb とかですかね.今は亡き Dir.each_left (5b9d15abdb389be4941850d9a471db102dadda22) とかがそうでした.ただ長く感じたので zip/ 直下に置きました.

コメントが欲しいです

具体的には何処でしょう

既存のモジュールの拡張なので、 *_extension.rb の形の名前になってると嬉しいです。

これまでは拡張では [class/module name]_[feature name].rb というネーミングだったところを,サブクラス拡張する場合は *_extension.rb というネーミングコンベンションにしようということですかね? (既存ライブラリのクラス名と同じファイル名は避けるべきというのは同意です.)

もし dst と entry.name が文字列だったら File.join のほうが良さそう?

一応 dst は Pathname がくることを想定しているので + で結合しますかね.このコメントに関連して思ったんですが,現在そこかしこで / を path separator として使っているので(例えば post.rb)ここだけ直してもしょうがない気も...

Logger を App の内部クラスにしてるのはなんででしょうか? Logger は App には依存していないように見えるので、独立させてもいいかと思いました。

そのまま外に出すとライブラリの Logger と名前被りが発生するというのと,他のいい名前が思い付かなかったので App を名前空間として利用しています.いい名前があれば App の外に出してもいいかもしれません.

この例外は誰がキャッチするんでしょうか?

誰もしません.強いていうなら Apache が起動した ruby が例外で死んだことになるので ISE (+ メッセージ?) が返ってきます.どうせ tester.cgi や interactor.cgi は外部には公開されていないので.(そして以前からこういう仕様だったので)

krtx commented 9 years ago

あ、慣習と言ったのは ruby でプログラム書くときの慣習くらいの意味で(それもよく分かってないんですが)automata がどうなってるかとかはあんまり考えずに発言してしまいました。個人的には zip/file_extension.rb に一票です。

chiro commented 9 years ago

これまでは拡張では [class/module name]_[feature name].rb というネーミングだったところを,サブクラス拡張する場合は *_extension.rb というネーミングコンベンションにしようということですかね? (既存ライブラリのクラス名と同じファイル名は避けるべきというのは同意です.)

はい、 _ext.rb とか _extensions.rb とか、いろいろ候補はありますが、そのへんはなんでもいいと思っています。 後から同じようにやりたい人はそれに合わせるでしょうし。

chiro commented 9 years ago

そのまま外に出すとライブラリの Logger と名前被りが発生するというのと,他のいい名前が思い付かなかったので App を名前空間として利用しています.いい名前があれば App の外に出してもいいかもしれません.

根本的には Automata 名前空間を導入して、みんなその下に移すべきなんでしょうね。

めんどくさい……

krtx commented 9 years ago

そのまま外に出すとライブラリの Logger と名前被りが発生するというのと,他のいい名前が思い付かなかったので App を名前空間として利用しています.いい名前があれば App の外に出してもいいかもしれません.

根本的には Automata 名前空間を導入して、みんなその下に移すべきなんでしょうね。

めんどくさい……

OK です。このままでいいと思いやす

skymountain commented 9 years ago

えっと、こういうの https://github.com/SoftwareFoundationGroupAtKyotoU/automata/blob/master/lib/reset.rb が欲しいです

わかりました.

あ、慣習と言ったのは ruby でプログラム書くときの慣習くらいの意味で(それもよく分かってないんですが)automata がどうなってるかとかはあんまり考えずに発言してしまいました。個人的には zip/file_extension.rb に一票です。

ruby の慣習はよく知らないですが,このプロジェクトの慣習としては(少なくとも機能を追加するぐらいであれば) [class name]/[feature name].rb となっているようです.例: string/random.rbfile/random_basename.rb, dir/each_leaf.rb なので,もし全てに *_extension.rb としたいのであれば,それはそれで別コミットにした方がよいと思います. (個人的には機能追加ぐらいなら suffix はいらないと思っていますが...)

はい、 _ext.rb とか _extensions.rb とか、いろいろ候補はありますが、そのへんはなんでもいいと思っています。 後から同じようにやりたい人はそれに合わせるでしょうし。

ではそうします.

根本的には Automata 名前空間を導入しれ、みんなその下に移すべきなんでしょうね。

めんどくさい……

はい,そうでしょうね.

krtx commented 9 years ago

もし dst と entry.name が文字列だったら File.join のほうが良さそう?

一応 dst は Pathname がくることを想定しているので + で結合しますかね.このコメントに関連して思ったんですが,現在そこかしこで / を path separator として使っているので(例えば post.rb)ここだけ直してもしょうがない気も...

文字列ではないということなので、了解しました。File.join に関しては path separator の問題だけではなく File.join("hoge", "fuga")File.join("hoge/", "fuga") でも "hoge/fuga" を返してくれるので、"#{a}/#{b}" で作るよりかは良いかと思って、そのようにコメントしました。

skymountain commented 9 years ago

文字列ではないということなので、了解しました。File.join に関しては path separator の問題だけではなく File.join("hoge", "fuga") も File.join("hoge/", "fuga") でも "hoge/fuga" を返してくれるので、"#{a}/#{b}" で作るよりかは良いかと思って、そのようにコメントしました。

なるほど,そういうことでしたか.だったら dir が Pathname であること依存しないだけそちらの方がよさそうです.

krtx commented 9 years ago

文字列ではないということなので、了解しました。File.join に関しては path separator の問題だけではなく File.join("hoge", "fuga") も File.join("hoge/", "fuga") でも "hoge/fuga" を返してくれるので、"#{a}/#{b}" で作るよりかは良いかと思って、そのようにコメントしました。

なるほど,そういうことでしたか.だったら dir が Pathname だること依存しないだけそちらの方がよさそうです.

ん、File.join は確か string だけだった気がするので、dir に Pathname が来ることが想定されているのであれば dir.join(path) とかが良さそうでしょうか? (未確認)

skymountain commented 9 years ago

ん、File.join は確か string だけだった気がするので、dir に Pathname が来ることが想定されているのであれば dir.join(path) とかが良さそうでしょうか? (未確認)

いや,dir に何がこようが File.join(dir.to_s, file.to_s) としようということです.

krtx commented 9 years ago

ん、File.join は確か string だけだった気がするので、dir に Pathname が来ることが想定されているのであれば dir.join(path) とかが良さそうでしょうか? (未確認)

いや,dir に何がこようが File.join(dir.to_s, file.to_s) としようということです.

了解しました

krtx commented 9 years ago

この例外は誰がキャッチするんでしょうか?

誰もしません.強いていうなら Apache が起動した ruby が例外で死んだことになるので ISE (+ メッセージ?) が返ってきます.どうせ tester.cgi や interactor.cgi は外部には公開されていないので.(そして以前からこういう仕様だったので)

んー、tester.rb 及び interactor.rb 自体で Internal Server Error を返す(つまり [500, {"Content-Type" => "text/plain"}, [msg 的なもの] ] 的なものを返す)方が RuntimeError で落ちてしまうよりは行儀がいいかなとちょっと思いましたが、外から見た挙動はそんなに変わらないので、今のままでも良いです

skymountain commented 9 years ago

諸々直しました.