Closed shogosanz closed 6 years ago
ありがとうございます、こちら大筋よさそうと思いました。
次の点なんとかなればマージしようと思います。
破壊的な操作と、非破壊操作を区別したい
exe/junoser-squash
とlib/junoser/squash.rb
だけでやるset interfaces xe-0/0/0
set interfaces xe-0/0/0 unit 0 address 10.0.0.1/30
は冗長ですがvalid です。意図を持って冗長にする場合も考えられるので、valid であるかぎり入力を尊重したい = 我々は入力の意図を汲めないので触らない方が良い、と思っています。
テストの追加
remove_subcommand
がO(n^2)
になっているの、工夫の余地ありなのですが…リファクタリングの前にテストあるとうれしいですね名前に関して追記
名前については、 compress
よりはsquash
のほうが良いと思いました。
compressならば、内容の圧縮→つまりdeleteやinsertなどの文も全部圧縮した損失ない最終型
はおっしゃる通りのイメージなのですが、「入力に対する破壊的操作ですよ」という点明示したいのです。
(とはいうもののsquash
もすごく気に入ってるわけではないので…マージまでに思いついたら引き続き名前募集です)
破壊的な操作と、非破壊操作を区別したい junoser はparser なので、非破壊を意図しています。 既存の名前空間に混ぜず、exe/junoser-squash とlib/junoser/squash.rb だけでやる
なるほど。 これについて、分離致しました。
は冗長ですがvalid です。意図を持って冗長にする場合も考えられるので、valid であるかぎり入力を尊重したい = 我々は入力の意図を汲めないので触らない方が良い、と思っています。
ref https://github.com/codeout/junoser/pull/8#issuecomment-352660621 のコメントも含めて考慮し、
新しいコミットでは、
set interfaces xe-0/0/0
set interfaces xe-0/0/0 unit 0 address 10.0.0.1/30
の場合は残し、
set interfaces xe-0/0/0 unit 0 address 10.0.0.1/30
set interfaces xe-0/0/0
の場合は削除する実装になっております。
名前については、 compress よりはsquash のほうが良いと思いました。 「入力に対する破壊的操作ですよ」という点明示したいのです。
なるほど。そういう意図があったのですね。理解しました。
ありがとうございます!
https://github.com/codeout/junoser/pull/10#issuecomment-359394369
で指摘さしあげたもののうち、一部commit いただきました。
次の点なんとかなればマージしようと思います。
ご対応ありがとうございます!
lib/junoser/squash.rb だけでやる
すいません!ファイルを変更しました!
テストの追加
新しいコミットでテストを追加しました。 他のテストを見習って書いてみましたが、うまく書けているでしょうか?
追記 テスト結果をペーストしておきます。
Loaded suite test/test_junoser-squash
Started
..
Finished in 1.970141 seconds.
------------------------------------------------------------------------------------------------------
2 tests, 2 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
------------------------------------------------------------------------------------------------------
1.02 tests/s, 1.02 assertions/s
ありがとうございます、よさそうなのでマージしますが、一点質問と一点お願いがあります。
exe/junoser-squash
に実行属性ついてますか? (手元では 644
でした)git rebase
して1 commit にしていただけますか?おお!ありがとうございます!
exe/junoser-squash に実行属性ついてますか? (手元では 644 でした)
chmod 755致しました。
git rebase して1 commit にしていただけますか?
commit汚くなりまして申し訳ありません。 1commitにまとめました。
パッチありがとうございました!
Junoser-Squashの実装
ref #8
わかりやすくするために、余計なものを取っ払ってsquashの機能のみに絞ったPRにしております。 頂いたgistに、あるset文の部分になるようなset文を除去するremove_subcommand関数を追加しました。
実行
名前に関して追記
今更名前についてまた議論するのはどうかとも思いましたが、単純に使っていて抱いた感想を言います。 squashはどうしても複数のものを一つにするという風に見える都合上、2つのファイルを統合するようなイメージを持ちました。 そこで、junoser-compressはどうでしょうか。 compressならば、内容の圧縮→つまりdeleteやinsertなどの文も全部圧縮した損失ない最終型へ というイメージがつきます。