[x64] x64 版でバージョン情報にアルファ版の表示を行う (再作成) #182
Conversation
m-tmatma
changed the title
| cmemMsg.AppendString( _T("(GitURL ") _T(GIT_URL) _T(")\r\n")); | ||
| #endif | ||
|
|
||
| // 段落区切り |
There was a problem hiding this comment.
#179 の対応ここでやったんですね…。
個人的にはこれだとバージョン文字列の全体の書式がぱっと見でわかりにくいのであまり好きではないですが、目的が達成できているのであれば構わないといえば構わないです。x64 マージを優先してレビューします。
以下のような文字列としてまとめていたほうが、どのような出力になるのかぱっと見でわかって良いと思っていました。
"v%d.%d.%d.%d %hs %hs\r\n"
"%hs"
There was a problem hiding this comment.
その書き方は条件コンパイルがない場合には
いいですが、条件コンパイルによって各要素が
増えたり減ったりする場合には難しいと思います
There was a problem hiding this comment.
そこは工夫次第ですね。将来どういう条件が加わるか分からない状態で将来のことを考えすぎることは自分はしないです。
sakura_core/version.h
Outdated
| @@ -26,7 +26,14 @@ | |||
| #define SPACE_WHEN_DEBUG "" | |||
| #endif | |||
|
|
|||
| #ifdef ALPHA_VERSION | |||
| #define ALPHA_VERSION_STR "Alpha Version" | |||
| #define ALPHA_VERSION_VER_STR " " ALPHA_VERSION_STR | |||
There was a problem hiding this comment.
実際のバージョンに使う文字列というような意味あいです。
|
PR本文内にもう少し要件を明記して欲しいです。 まぁソース見れば分かるといえば分かるのですが、実装者とレビュアーの間で要件の認識が合っている上でレビューをしないと中途半端な（というかたぶんコミュニケーションコストの大きな）レビューになります。 今回の変更は「ダイアログ上のバージョン情報」および「EXE, DLL 上のバージョン情報」の変更が要件ということで合っていますか？ |
sakura_core/StdAfx.h
Outdated
| @@ -16,6 +16,16 @@ | |||
| #define STRICT 1 | |||
| #endif | |||
|
|
|||
| #if _WIN64 | |||
| #define ALPHA_VERSION | |||
| #endif | |||
There was a problem hiding this comment.
StdAfx.h はリソースファイル側にはインクルードされないという認識です。
これだと ALPHA_VERSION 定義が .rc に反映されないはず。
|
EXE, DLL のバージョン情報文字列に「Alpha」が入っていないのは意図通りですか？
|
|
最低限、バージョンダイアログに Alpha 表示が入っていればユーザ確認手段は揃っている（EXE や DLL のバージョン情報を見に行く人などめったにいない）ので x64 を master に統合する上では今のままマージしてしまうのも構わないと思っていますが、これを修正するしないは @m-tmatma さんの判断にお任せします。 |
|
気になるなら直します。 |
|
個人的には書き方についてのこだわりで PR は修正してもらうまでもないと思っていて（変える必要があればあとから変えれば良い）、
ここの意図の確認が一番大事なところです。 |
|
「EXE, DLL のバージョン情報文字列に「Alpha」が入っていない」のが意図通りではない場合に
これは自分としてはどちらでも良いです。 |
|
タイトルとバージョンダイアログを対応すればいいと思って考えていました。 |
|
どうあるべきか、ではなく、「この PR の目的（この PR によって実現されること）は何か」を書いていただきたいです。 |
|
「タイトルとバージョンダイアログの記載内容の変更」がこの PR の要件ということでよろしいですか？ |
|
32bit Debug
32bit Release
64bit Debug
64bit Release
32bit Debug (Resource)
32bit Release (Resource)
64bit Debug (Resource)
64bit Release (Resource)
|
|
リソースの対応が要件から漏れていたので対応しました。 |
sakura_core/config/app_constants.h
Outdated
| #endif | ||
|
|
||
| //��:UNICODE�f�o�b�O��_T("sakura(�f�o�b�O��)") | ||
| #define _GSTR_APPNAME_(TYPE) _APP_NAME_(TYPE) _APP_NAME_2_(TYPE) _APP_NAME_3_(TYPE) |
There was a problem hiding this comment.
メモ：このあたりは別件として version.h にまとめて整理することを考えています。たぶん自分がやります。
…about-dialog-from-x64 [x64] x64 版でバージョン情報にアルファ版の表示を行う (再作成)
x64 版でバージョン情報にアルファ版の表示を行う
#174 を捨てて x64 ブランチを元に作成し直した。