行毎ではなく一括で線を描く変更によって引き起こされた表示の不具合を起こさないように元に戻す

[beru]

PR の目的

#1065 (comment) で @usagisita さんが報告してくれた問題を解消する為に下記の変更を revert します。

• ノート線描画、指定桁縦線描画、折り返し桁縦線描画、を行毎ではなく一括で行うように変更 #1065 ノート線描画、指定桁縦線描画、折り返し桁縦線描画、を行毎ではなく一括で行うように変更
• ノート線描画を少し分かりやすくする #1066 ノート線描画を少し分かりやすくする

カテゴリ

• 不具合修正

関連チケット

#1071

[AppVeyorBot]

✅ Build sakura 1.0.2304 completed (commit d940584202 by @beru)

[berryzplus]

趣旨

想定しない差分が出ています。
意図通りの変更ができているか確認お願いします。

こちらでの確認内容

1. コミットログとPR本文の説明が一致しているか確認しました。
   ノート線描画、指定桁縦線描画、折り返し桁縦線描画、を行毎ではなく一括で行うように変更 #1065 と ノート線描画を少し分かりやすくする #1066 の revert と説明されているので、これは問題ないと思います。
   
2. ノート線描画、指定桁縦線描画、折り返し桁縦線描画、を行毎ではなく一括で行うように変更 #1065 適用前 と PR 適用後を比較して、差分が適切か確認してみました。
   これがNGです。5ファイル変更された差分になっていますが、ノート線描画、指定桁縦線描画、折り返し桁縦線描画、を行毎ではなく一括で行うように変更 #1065 と ノート線描画を少し分かりやすくする #1066 を revert すると差分は3ファイルになるはずだからです。
   
   ちなみに上の画像は下図のように範囲差分をとって表示させたものです。
   

#1067 の変更範囲で差分をとる

#1069 の差分は 1ファイルのみ なので 5ファイル 変更されているのはおかしい・・・はず。

[berryzplus]

補足。

構造的に

• 1064 をマージした
  • 1065 をマージした　←　これを revert したい
  • 1066 をマージした　←　これも revert したい
  • 1067 をマージした　←　2ファイル変更。これは残していいので残したい
  • 1068 はキャンセルされた...orz
  • 1069 をマージした　←　1ファイル変更。これは残していいので残したい

1064マージ直後基準でみたとき、このPRが目的通りに作成されているとするなら、
1067 + 1069 の変更で 3ファイル になるはずなんだけど、
比較したら 5ファイル変更されていることになっていました、ということを言っています。

PRの目的には納得しているので、2ファイル分の変更を元に戻すコミットを追加で積むか、それらの変更をrevertしない理由の説明があればとくに問題ない PR だと思っています。

[beru]

#1065 の変更で害がなさそうなのは元に戻してないからそういう差分になるのかもしれないですね。

手作業で必要と思われる箇所だけを変更したので。。

確認しづらいから該当PRは全部まとめて revert してくれというリクエストがあればそうします。

[berryzplus]

#1065 の変更で害がなさそうなのは元に戻してないからそういう差分になるのかもしれないですね。

それをやったら revert と言えないと思います。

Revert のレビューのつもりだったので詳しく見ていませんが、1067の変更が1065の変更に依存しているのであれば、それも含めてrevert すべきだと思います。

[berryzplus]

確認しづらいから該当PRは全部まとめて revert してくれというリクエストがあればそうします。

影響を受ける PR はまとめて revert してください。

ビルドの範疇では、1067は1065に依存してないようでした。
1065の一部を「問題なし」として活かすことに異論はありません。
「revertします」で「一部はrevertしません」に引っかかってるだけです。

「対象になるコードのうち、これは影響ないので残したい」は revert 自体とは別で検討したいです。そうしなかった場合、何をrevertして何をrevertしなかったのか分からなくなります。

「revertしない」ってことは差分が出ないってことです。機械的にrevertせずに「一部を残す」という判断をした PR には「その判断は適切か？」という観点のレビューが必要になります。しかし、差分が出ない箇所に指摘を付ける方法はないんです。それは困る・・・。

蛇足ですが 1065, 1066 だけを正確に revert するPRを作成する方法

• コミットログ で 1064 merged のコミットを右クリックして Reset revert_DispWrapLine_DispNoteLines to this
• 表示された Reset ダイアログで Reset Type を Hard にして OK
• コミットログ で 1069 merged のコミットを右クリックして Reset revert_DispWrapLine_DispNoteLines to this
• 表示された Reset ダイアログで Reset Type を Mixed にして OK
• Gitコミットダイアログを表示する
• 対象外ファイル funccode_x.hsrc, CRuler.h, CRuler.cpp を選択して Revert
• コミットメッセージを入力して OK

[beru]

#1065 と #1066 のコミットは全て選択して revert しました。

[AppVeyorBot]

✅ Build sakura 1.0.2306 completed (commit 7ec20fcf11 by @beru)

[berryzplus]

問題ないと思います。

05fa19e からの差分をとって、1065と1066の変更が含まれていないこと、1067と1069の変更が含まれていることを確認しました。

[beru]

レビューありがとうございました。Merge します。