Build and run tests on MinGW environment.

[ds14050]

#579 の続きです。MinGW ビルドでテストプログラムを作成して実行します。AppVeyor が×を出すはずですが #559 (comment) で予告していた通りであり、この PR の問題ではありません。

sakura_core/Makefile に対する修正はローカルの cmd.exe が / 区切りのパスを解釈しなかったことへの対処です。

tests/create-project.bat で cmake へのオプションを変えていますが、これは cmake 3.12 のドキュメントや cmake --help、cmake --build を読んでも -B オプションの説明が見つからなかったからです。-H オプションについては --help, -help, -usage, -h, /? と同じだと書かれていました。Software pre-installed on Windows build VMs | AppVeyor によると AppVeyor にプリインストールされているのは CMake 3.12.2 です。

[m-tmatma]

tests/create-project.bat で cmake へのオプションを変えていますが、これは cmake 3.12 のドキュメントや cmake --help、cmake --build を読んでも -B オプションの説明が見つからなかったからです。

これは公然の隠しオプションです。

ドキュメントに記載するように試みた人がいたようですが、拒否されたようです。
https://stackoverflow.com/questions/31090821/what-does-the-h-option-means-for-cmake
https://cmake.org/pipermail/cmake-developers/2016-June/028843.html
https://cmake.org/pipermail/cmake-developers/2016-June/028844.html

便利なんだけど……

[ds14050]

これは公然の隠しオプションです。

では戻しておきます。

[m-tmatma]

#579 の続きです。MinGW ビルドでテストプログラムを作成して実行します。AppVeyor が×を出すはずですが #559 (comment) で予告していた通りであり、この PR の問題ではありません。

別 PR で _swprintf_p に対するリンカーエラーを修正する予定ですか?

[ds14050]

別 PR で _swprintf_p に対するリンカーエラーを修正する予定ですか?

@berryzplus さんにお任せしたいなと思っています。

別の目的のブランチで修正を試みたのですが、// でコメントアウトしている２、３行のテストが、Win32 ビルドでは通るのに MinGW ビルドでは通らないという状態です。なのであまり考えたくないな、と。

[m-tmatma]

別 PR で _swprintf_p に対するリンカーエラーを修正する予定ですか?

@berryzplus さんにお任せしたいなと思っています。

だとしたら、この PR のタイトルに WIP をつけた上で、別チケットを作って、
その対応待ちと表明しておいてほしいです。

[ds14050]

だとしたら、この PR のタイトルに WIP をつけた上で、別チケットを作って、
その対応待ちと表明しておいてほしいです。

いえ、自分の作業は済んでいます。AppVeyor のビルドが通らなくてもマージはできますし、ビルドが通ってからマージするというのはレビュアの判断です。

[m-tmatma]

ビルドが通ってなくて実行確認ができない状態では、
作業が終わっているとは言えないです。

[ds14050]

待つだけですよ？ そしてそれはレビュアの判断です。

[m-tmatma]

待つだけですよ？ そしてそれはレビュアの判断です。

待つだけというより、待たないといけないというのを表明するのが重要です。

いえ、自分の作業は済んでいます。

何をこの PR のターゲットと想定しておられるのか不明ですが、
何か作業をして、その結果ビルドが通って実行して問題ないことを確認するというのが
作業終了の条件です。

その意味でいうとこの PR は完了していないです。

AppVeyor のビルドが通らなくてもマージはできますし、ビルドが通ってからマージするというのはレビュアの判断です。

Appveyor のビルドが通らない状態で、マージはするべきではないです。
もしそんなことをしたら、CI を使っている意味がないです。

何か、時間のかかる問題があって、すぐには解決できないのであれば、
例えば暫定的に appveyor のビルド対象から外す、というのはありですが。

[berryzplus]

別 PR で _swprintf_p に対するリンカーエラーを修正する予定ですか?

@berryzplus さんにお任せしたいなと思っています。

対応しまーっす。
このPRのレビューはそこ以外の観点で進めておく、ということでよいかと思います。

PRの趣旨について、現段階で迷いがあります。
このPRで実現したいことは、MinGWのビルド成否をappveyorの成否に反映する、だと思います。
つまり、MinGWビルドが通らないとappveyorが通らないことになります。

それでいいんだっけ？というのが現段階の迷いです。

[m-tmatma]

#593 をマージしました。

[ds14050]

PRの趣旨について、現段階で迷いがあります。
このPRで実現したいことは、MinGWのビルド成否をappveyorの成否に反映する、だと思います。
つまり、MinGWビルドが通らないとappveyorが通らないことになります。

それを実現するのは #590 です。

この PR の主旨は MinGW ビルドでスキップされているテストをスキップせずに実行しようぜっていう、特に MinGW ビルドの存在意義に思いを巡らせたわけではない、単に足りないところを補ったというだけのもくろみです。

[ds14050]

何か作業をして、その結果ビルドが通って実行して問題ないことを確認するというのが作業終了の条件です。

その意味でいうとこの PR は完了していないです。

自分のアカウントでは AppVeyor のビルドが通るのを確認しています。その後無関係なテストコードにまつわる変更を外して PR を出しました。

自分は用意していますが、PR を出す人すべてに AppVeyor の利用を求めるのはやりすぎです。であれば、PR を出してみなければ AppVeyor のビルドが通るかを確認することは不可能です。

AppVeyor のビルドが通らなくてもマージはできますし、ビルドが通ってからマージするというのはレビュアの判断です。

Appveyor のビルドが通らない状態で、マージはするべきではないです。
もしそんなことをしたら、CI を使っている意味がないです。

それこそがレビュアの判断だと言っています。その判断を否定はしませんが、AppVeyor はツールのひとつです。絶対視はしません。便利につかいこそすれ、それなしでは何もできないとは言うべきではないでしょう。そのことが直ちに、ブランチをフェッチしてローカルで手修正を加えてビルドして PR の内容を確かめるべきだということを意味するわけではありません。berryzplus さんの修正と AppVeyor の報告を待つつもりでいます。ビルド通ってねーじゃん、と m-tmatma さんに一瞥ももらえなくても、それはそれでしかたのないことだと思っています(が、嫌なので断り書きを添えました)。

[berryzplus]

PRありがとうございます。

このPRでMinGWのテストが有効になるんですね。ちょっと勘違いしました。
MinGWのビルドエラーを解消するPRがマージされたのでrebaseをお願いしたいです。

たぶん本筋外ですが、指摘あるので回答のほどお願いします。

[ds14050]

MinGWのビルドエラーを解消するPRがマージされたのでrebaseをお願いしたいです。

マージ後に AppVeyor でリビルドをかけて無事通りました。このままでマージできることも確認しています。「Able to merge. These branches can be automatically merged.」「Merging can be performed automatically with 1 approving review.」

rebase が必要ですか？

[berryzplus]

rebase が必要ですか？

しなくて良いと思います。
rebaseしないと無理だと思ってました。

[m-tmatma]

tests/build-and-test.bat がコンフリクトしてしまっているので
解決お願いします。

[m-tmatma]

あと指摘に対応して push したら、しばらく経っても反応ないときは
明示的にレビュー依頼してください。

[ds14050]

あと指摘に対応して push したら、しばらく経っても反応ないときは明示的にレビュー依頼してください。

いい心がけだと思います。特に国が異なると、せっついて初めて事が動き出すということがあるようです。待ってると待ちぼうけ、みたいな。でも待ちますよ。忘れられるなら不要だったということです。

[ds14050]

push したことや、master へのマージに伴うコンフリクトの発生では通知が飛ばず、気が付きにくいことは心にとめておきます。

[m-tmatma]

push したことや、master へのマージに伴うコンフリクトの発生では通知が飛ばず、

通常の push だと通知が飛びますが、rebase による push では飛ばないみたいです。

[m-tmatma]

対応ありがとうございます。

[ds14050]

これは今後に希望することなのですが、自分は個人の責任範囲にかなり強いこだわりがあります。私の仕事、あなたの仕事、という切り分けのことです。自分はマージボタンを押す権限を持っていますから、自分のコードを master ブランチへマージする行為は自分の責任でもって行いたいと考えています。これはそのコードにより起こった問題へは第一義的に対応するということも意味しています。配慮していただければ幸いです。急ぎの場合や何日も不在の場合はもちろんその限りではありません。

[berryzplus]

#309 を早く閉じたかっただけで、他意はないと思います。
急ぎでない限り、approveとmergeは別の人が行うのがベターだと思ってます。
ぼくがmergeしてたかも知らんのでドキッとしました。

[ds14050]

他意はないと思います。

特に不愉快な意図を感じとったというわけではありません。自分がこのような分担でやりたいと考えているということです。他のメンバーの PR に対して自分が Approve はするけれど Merge まではしないのも同じ考えに基づいています。つまり、最終行為者は本人であるべきだという考えです。

[ds14050]

rebase が必要ですか？

しなくて良いと思います。
rebaseしないと無理だと思ってました。

他の件で検索していたところ必ず rebase を行う派の意見が見つかりました。曰く、ブランチの系統樹が伸びて複雑に絡み合うのを避けリバートがしやすくなるとか。マージコミットを作成する場合にはそういう考慮があるみたいです。

[ds14050]

そういう考慮

これはコンフリクトの解消を伴う単純でないマージを想定したものであって、GitHub を前提とした作業フローには当てはまらないかもしれません。