ぴーさんログ

だいたいXamarin.Formsのブログ

Xamarin.Formsにコントリビュートしよう

Xamarin.Forms GitHub

先日、Xamarin.FormsのVisualSateManagerの解説を書いている途中で、バグを見つけて修正PRを投げました。 せっかくなのでどんな感じだったのか共有します。レッツ貢献！

この件。

概要

  • バグを見つける
  • ソースコードの問題個所を確認する
  • Reproductionを用意する
  • Issueを登録する
  • Pull Requestする
  • 自動テストがコケる
  • 取り込まれる

バグを見つける

Xamarin.Forms 3.0.0-pre2を使って、暗黙的に変化するVisualStateのサンプルコードを書いていたところVisualElement.IsFocusedが変化したときの挙動がおかしい事に気づきました。

ソースコードの問題個所を確認する

条件演算時が逆になってます。修正も簡単なのでバグ報告と一緒に修正PRも投げることにしました。

// 実際のコード
VisualStateManager.GoToState(element, isFocused
    ? VisualStateManager.CommonStates.Normal
    : VisualStateManager.CommonStates.Focused);

// 正しくはこう
//VisualStateManager.GoToState(element, isFocused
// ? VisualStateManager.CommonStates.Focused
// : VisualStateManager.CommonStates.Normal);

Reproductionを用意する

バグ報告にあたってReproduction、つまり最小の再現環境を用意します。 バグ報告者、Reproductionは義務です。え、Reproductionが無い？ZAPZAPZAP!次のバグ報告者はきっとうまくやってくれるでしょう。

もともとサンプルコードを書いていたところだったので、関係ないところを削ってGitHub置いておきます。

Issueを登録する

Xamarin.FormsのリポジトリにはIsuue登録のテンプレートがあります。

### Description

説明を書きます。

### Steps to Reproduce

再現手順を書きます。

1. 
2. 
3. 

### Expected Behavior

期待する動作を書きます。

### Actual Behavior

実際の動作(バグ挙動)を書きます。

### Basic Information

バグが発生する環境のバージョンなどを書きます。関係ないのは削ってもOK。

- Version with issue:
- Last known good version:
- IDE:
- Platform Target Frameworks: 
  - iOS:  
  - Android: 
  - UWP:  
- Android Support Library Version:
- Nuget Packages:
- Affected Devices:

### Screenshots

見た目に関する問題の場合はスクリーンショットがあると良い。

### Reproduction Link

最小の再現環境へのリンクを張ります。

実際に登録したIssueがコチラ

バグが報告されると中の人がTriageというGitHubのProjectsに乗せて行程管理するみたい。 「Ready For Work」にあるやつは修正PR受付中かな？

Pull Requestする

Pull Requestにもテンプレートがあります。

### Description of Change ###

変更内容の説明を書きます。

### Bugs Fixed ###

修正するIssueの番号を書きます。

### API Changes ###

Publicなクラスやメソッドが増えたり減ったりシグネチャが変わったりする場合は列挙します。何も無い場合はNoneで。

### Behavioral Changes ###

挙動が変わる場合は列挙します。何も無い場合はNoneで。

### PR Checklist ###

PRするときにやることリスト。

- [ ] Has tests (if omitted, state reason in description)
- [ ] Rebased on top of master at time of PR
- [ ] Changes adhere to coding standard
- [ ] Consolidate commits as makes sense

(「Checklistは自己申告なのか？」とか「あからさまにbool逆だっただけなのにテストいるのか？」と悩んだ結果、とりあえず未チェックのままPRしたら結局そのままマージされてしまった。)

実際のPull Requestがコチラ

masterリポジトリ充てにPRしたところ、リリース用ブランチ(3.0.0)宛てに変えてくれと言われる。分岐点までresetしてから3.0.0ブランチの先頭にrebaseし、GitHubの機能で宛先を3.0.0ブランチに変更。

自動テストがコケる

気が付いたら自動ビルドが走っていて(中の人が手動でキックするっぽい？)、数十分後に自動テストが走っていました。 次の日、「UITestxxxxが失敗した」とコメントがついてました。

取り込まれる

件のUITestを確認すると私の修正とは関係なく(そりゃそうだ)、typo修正が不完全でAutomatonId不一致で失敗しているだけでした。 その旨を返答すると数時間後にマージされました。(見返すと英文間違っててヤバイ)

反省

  • 一行程度の修正でも横着せずにブランチを切る。
    • rebaseが面倒になった。
  • CheckListは自分でチェックしていい。
  • 雰囲気英文は見直す。

おまけ