kamipoさんのアドバイスと拠点を跨いだ勉強会
この記事は京都開発拠点アドベントカレンダー 18日目の記事です。
マネーフォワードには開発拠点が複数ありますが、技術情報はSlackで議論・共有されています。
今日はそんなSlackの中から、東京拠点と京都拠点を跨いで開催した「Railsへコントリビュートする勉強会」について、東京拠点のアルパカ隊長が紹介します。
はじまりのSlack
ふとした時に、社内SlackのRubyチャンネルでこんな会話がありました。
@alpaca-tc
程よい難易度で、kamipoさんが「これ自分がやらんでもええやろ」みたいなissueがあれば
このslackに投げてもらって
誰かシニアエンジニアがサポートしつつ、新卒がRailsコントリビュートするみたいなのやりたい:eyes:
すると、kamipoさんからすぐ解決できるものは見た瞬間に直してしまっていると前置きがあった上で、 こんなissueを教えてもらいました。
@kamipo
簡単じゃないけど直したほうがいいと思ってるやつだとたとえば rails/rails#35204 とか。
これはautomatic_inverse_ofがバグっててinverse_ofって一対一の関係になるはずなのに
belongs_to :user と belongs_to :writer の両方ともinverse_ofが :user と誤認されるってバグで、
起きてることに気づきづらいから直したほうがいいんだけど
inverse_of: false すれば回避できるから優先順位が低い、とかそういう系のはいっぱいある。
ふーむ、パッと見はよく分かりません。程よい難易度を感じます。
追加でかなり詳しいアドバイス(ほぼ答え)をいただいて、早速チャレンジですヽ(・∀・ )ﾉ
拠点を跨いで勉強会を開催
昨今のコロナ禍によってオンライン化が一気に進みました。(僕も、今年は数回しか出社していません)
おかげで、拠点間を跨ぐオンライン勉強会も突発的に開催できるようになりました。
当日の流れ
今回は、京都の開発メンバーを含めた数人のメンバーを募り、Discordで話しながらkamipoさんのアドバイスを紐解いていくことになりました。
まずはセットアップ
Contributing to Ruby on RailsからActiveRecordに関するセットアップ手順を抜粋して、各自でローカル環境をセットアップしていきます。
その上で、先ほどのissueを再現するテストコードを書いて、テストが失敗する状態を再現しました。
続いて調査
そして、全員の環境でテストが失敗する状態になったら、kamipoさんにいただいたアドバイスを調査していきます。
このissueを参照してるここ rails/rails#36708 でも言及してるんですが、
automatic_inverse_ofは改善可能で改善すれば自ずとこの問題も解決されます。具体的には :foreign_key オプションをinverse_of判定に含めることでこれまで判定不能だったinverse_of判定も可能になります。
このissue自体はforeign_key未指定の同一クラス参照してるassociationがある場合にforeign_keyの一致を判定せずにクラスが一致するかどうかだけでinverse_of判定してることが問題なのだと思います。
ざっくり言うと、このissueはモデルの関連を指定した時に、判定の条件漏れがあることが原因のようです。
みんなでActiveRecord::Reflection#automatic_inverse_ofあたりを読み進めて、inverse_ofの判定処理に誤りがあることを発見しました。
PRが完成
わいわい話しながらPRができたところで、kamipoさんにPRを見てもらって更にアドバイスをいただきました。
- よほどのことがない限り1PR 1commitになるようにするのがのぞましい contribution guide
- レビュアーがまだ知らない情報は、ぜんぶPRに含めてくれるほうがレビューのハードルが下がるので頑張って欲しい
- レビュアーが、わざわざこれは正しい変更だと解説コメントしなくてもいいぐらい書くといい
- 完全さよりもコンテキストを共有する気持ちが重要
貴重なアドバイスをいただきながら、完成したPRがこちらです。
このPRは無事マージされ、Rails6.1.0でリリースされています🎉
Slackでの四方山話
今回は勉強会の話を抜粋しましたが、普段からSlackでは様々なトピックの技術系の話題がなされています。
個人的にも、kamipoさんにアドバイスをいただけたおかげで、直近1ヶ月で4つほどPRを送ることができました。
ひとりでOSSへ取り組むのは難しさがありますが、Slackで議論ができるとハードルがグッと下がって踏み出しやすくなりますね。
まとめ
今回の勉強会の件もそうですが、弊社では拠点に関係なくこういった技術の話題でワイワイできます。 ぜひ皆さんも遊びにきてくださいヽ(・∀・ )ﾉ
まとめ 「kamipoさんはすごい人」
(※kamipoさんは弊社社員ではなく、善意でSlackのチャンネルに入ってくださっています。)