Kengo's blog

Technical articles about original projects, JVM, Static Analysis and TypeScript.

patch-erの心構え?

args4jにpatchを送りました。まさか著名なライブラリのバグ容疑者を発見するとは思わず。「Argumentアノテーションを使い」「不正な値を渡す」という2因子の現象ですね。

23時頃に意図しない動作に気づき、そのまま寝たものの気になって4時に起きて書いてしまいました。こういうのほっとくのは精神衛生上よくないですね。


世間一般の開発者の皆さんは、こういうpull requestやpatchを送る際はどのようなことに気を配っていらっしゃるのでしょう?
私の理想は変数名やクラス名、コミットログといったものがプロダクトに"溶け込む"ことなので、命名周りに一番注意しています。今回EnumArgumentとEnumArgumentTestというクラス名を選んだのは、既に存在するEnumAttributeとEnumAttributeTestというクラスを真似たためです。当初はEnumAttributeArgumentという案もあり、その方がクラス名として自身をよく表しているかもと思ったのですが、そうするとIDE表示が汚くなる*1ので避けています。
既存コードを大幅に変えていいならば、EnumAttributeをEnumOptionに改名してEnumAttributeTestに今回追加したテストケースをすべて入れるというのもアリですね。あるいはEnumOptionTestとEnumArgumentTestにするか。

たぶんこのあたりはこだわりの領域で、自分の提案がどんなに効果的かPRする方が重要なのでしょう。テストケースを送るとか、パフォーマンス計測結果を送るとか。そのあたりはマナーというか当然なので、逆に語られないのかもしれません。

追記

2.0.19でマージして頂きました。

しかしassertThatを使う所でassertTrueしてるとかそもそも英語がアレとか、見返すと相当ダメなコードを送ってますね……。まぁこの短期間にそういうことに気付けるように成長した、ということでプラスに考えましょう。

*1:EnumAttributeとEnumAttributeTestの間にEnumAttributeArgumentが入ってくる!