Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

バージョン情報をリファクタリングする #1738

Merged
merged 4 commits into from
Oct 11, 2021
Merged

バージョン情報をリファクタリングする #1738

merged 4 commits into from
Oct 11, 2021

Conversation

berryzplus
Copy link
Contributor

@berryzplus berryzplus commented Oct 9, 2021 •
edited
Loading

PR の目的

バージョン情報をリファクタリングして課題を解決することが目的です。

カテゴリ

  • 仕様変更
  • 不具合修正
  • リファクタリング

PR の背景

Appveyorのビルドが遅いので、GitHubActionsにメインCIを移行したいです。

メインCIを移行するには、バージョン番号の仕様を変える必要があります。
現在のバージョン番号仕様は v2.4.2.(Appveyorのビルド番号) です。
Appveyorのビルド番号はGitHubActionsからは取得できないので、ここを変える必要があります。

PR のメリット

PR のデメリット (トレードオフとかあれば)

仕様・動作説明

バージョン番号の振り方を変える修正なので「仕様変更」としています。
バージョン情報が見切れている不具合に対策を打つので「不具合修正」としています。
アプリ名の組み方を変えるので「リファクタリング」としています。

変更内容 アプリ名 バージョン番号
変更前 sakura(デバッグ版) v2.4.2.1234(末尾はCI別のビルド番号)
変更後 サクラエディタ開発版(64bitデバッグ) v.2.4.2.1234(末尾はGitの累積コミット数)

PR の影響範囲

  • 「アプリ名」「バージョン情報」を表示する機能に影響します。
    • アプリのタイトルバー表示内容
    • メッセージボックスのタイトル
    • バージョン情報ダイアログの表示内容
  • リソースに埋め込まれる「製品名」「バージョン情報」に影響します。

テスト内容

ビルドしたsakura.exeを起動して、タイトルバーの「アプリ名」が反映されていることを確認しました。

テスト1

手順

関連 issue, PR

参考資料

@berryzplus berryzplus added management 運営に関する話題 【ChangeLog除外】 specification change ■仕様変更 CI appveyor など CI 関連 【ChangeLog除外】 🌏Internationalization 🇯🇵 🇺🇸 🇨🇳 🇹🇼 国際化対応 azure pipelines appveyor local build GitHub Actions GitHub Actions関連 labels Oct 9, 2021
@AppVeyorBot
Copy link

✅ Build sakura 1.0.3943 completed (commit b27da72582 by @berryzplus)

@berryzplus
Copy link
Contributor Author

Azure PipelinesのMinGWビルドで障害発生しました。
そして、検証中に #1740 が発覚しました。
もうちょっとやり方考えてみます。

@berryzplus berryzplus force-pushed the feature/refactoring_version_info branch from 7e06a83 to c800f69 Compare October 9, 2021 16:02
@AppVeyorBot
Copy link

❌ Build sakura 1.0.3944 failed (commit 7b59a5f7f9 by @berryzplus)

@berryzplus berryzplus force-pushed the feature/refactoring_version_info branch from c800f69 to 79e07fa Compare October 9, 2021 16:14
@AppVeyorBot
Copy link

✅ Build sakura 1.0.3945 completed (commit b6677d5027 by @berryzplus)

@ghost
Copy link

ghost commented Oct 10, 2021 •
edited by ghost
Loading

git fetchして確認しました。

dlg_versioninfo_japanese
dlg_versioninfo_english

バージョン表記とビルド環境名（BUILD_ENV_NAME）の間にスペースを入れることはできませんか？
ウィンドウタイトルでは2.4.2.0・ファイルプロパティの製品バージョンは2.4.2.0 (79e07fa5b)と表示されますが、バージョン情報ダイアログだけ「ビルド環境名をバージョン表記に含む」ような表示なのは違和感があります。
また、接頭辞「v」が付くのもこのダイアログだけですね。

英文では開き括弧の前と閉じ括弧の後にはスペースが来るので、正式リリース版で英語表示の時に製品名の括弧の前にスペースがない（SAKURA Editor(32bit)）のも違和感がありますね。
知っておきたい英文のスペースのあけ方 - yanok.net

@ghost
Copy link

ghost commented Oct 10, 2021

#759 もこのPRで解決するという理解で良いですか？

@berryzplus
Copy link
Contributor Author

よし、ビルドは通った・・・。

  • HeaderMakeがアンチウィルスアプリに有害と判定される #1740 の影響で、うちのローカルでは MinGW ビルドが常に失敗するようになっています。
    （HeaderMake.exeがウイルスとして検知され、ビルド中に削除されるから）
  • SonarCloudでリテラル文字列の結合がBugsとして誤検知されていたのをスルーしました。
  • SonarCloudでversion.hのdefineに「constexprを使え」の警告が出ていたのを won't fix にしました。

@berryzplus berryzplus marked this pull request as ready for review October 10, 2021 03:48
@berryzplus
Copy link
Contributor Author

キャプチャ貼りありがとうございます。

バージョン表記とビルド環境名（BUILD_ENV_NAME）の間にスペースを入れることはできませんか？ ウィンドウタイトルでは2.4.2.0・ファイルプロパティの製品バージョンは2.4.2.0 (79e07fa5b)と表示されますが、バージョン情報ダイアログだけ「ビルド環境名をバージョン表記に含む」ような表示なのは違和感があります。

できます。
ただ、環境名の表示位置は「そもそもここでいいのか？」の課題があります。

  1. バージョン情報ダイアログのバージョン表記付近（いまここ）
  2. バージョン情報ダイアログのコンパイル情報に含める（ここかも？と思っています。）
  3. タイトルリソースに含める
  4. バージョンリソースに含める

また、接頭辞「v」が付くのもこのダイアログだけですね。

気付いていませんでした。
言われてみれば「なんでやねん！」ですね。

英文では開き括弧の前と閉じ括弧の後にはスペースが来るので、正式リリース版で英語表示の時に製品名の括弧の前にスペースがない（SAKURA Editor(32bit)）のも違和感がありますね。

すみません、たぶん作業ミスです。意図的にスペースを入れたつもりでした・・・。

@berryzplus berryzplus marked this pull request as draft October 10, 2021 03:56
@berryzplus
レビュー指摘対応
4d906ca 
・BUILD_ENV_NAMEの前に空白を入れる
・英語版丸括弧の前に空白を入れる
・「v」を変更前コメントに合わせ「Ver. 」にしてみる
@AppVeyorBot
Copy link

❌ Build sakura 1.0.3946 failed (commit 7f8b9152f9 by @berryzplus)

@ghost
Copy link

ghost commented Oct 10, 2021

ただ、環境名の表示位置は「そもそもここでいいのか？」の課題があります。

  1. バージョン情報ダイアログのバージョン表記付近（いまここ）
  2. バージョン情報ダイアログのコンパイル情報に含める（ここかも？と思っています。）
  3. タイトルリソースに含める
  4. バージョンリソースに含める

バージョンの情報ではないし、かといってコンパイラの情報でもなさそうですし、新たにBuild Environment: とセクションを取るとか……悩ましいですね😅

@AppVeyorBot
Copy link

✅ Build sakura 1.0.3946 completed (commit 7f8b9152f9 by @berryzplus)

berryzplus
berryzplus commented Oct 10, 2021
View reviewed changes
sakura/githash.bat Outdated Show resolved Hide resolved
@berryzplus berryzplus force-pushed the feature/refactoring_version_info branch from 28dfa70 to d51ad22 Compare October 10, 2021 11:42
@AppVeyorBot
Copy link

✅ Build sakura 1.0.3948 completed (commit 2cacc78bf1 by @berryzplus)

@AppVeyorBot
Copy link

✅ Build sakura 1.0.3949 completed (commit 36bb2c94c8 by @berryzplus)

@berryzplus berryzplus force-pushed the feature/refactoring_version_info branch from d51ad22 to 9983279 Compare October 10, 2021 12:30
@AppVeyorBot
Copy link

✅ Build sakura 1.0.3950 completed (commit f5d8b98d36 by @berryzplus)

@berryzplus berryzplus force-pushed the feature/refactoring_version_info branch from 9983279 to 7bf6546 Compare October 10, 2021 13:15
@berryzplus berryzplus force-pushed the feature/refactoring_version_info branch from 7bf6546 to 7f79246 Compare October 10, 2021 13:31
@AppVeyorBot
Copy link

✅ Build sakura 1.0.3951 completed (commit 188e2c7714 by @berryzplus)

@AppVeyorBot
Copy link

✅ Build sakura 1.0.3952 completed (commit 8c1a0e8d90 by @berryzplus)

@berryzplus
Copy link
Contributor Author

CIビルドをできるだけ早く済ませるためにshallow cloneしてるんですが、累積コミット数を取るにはマズいっぽい・・・。

@sonarcloud
Copy link

sonarcloud bot commented Oct 10, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@AppVeyorBot
Copy link

✅ Build sakura 1.0.3953 completed (commit 5fd3ae19f8 by @berryzplus)

@berryzplus berryzplus marked this pull request as ready for review October 11, 2021 00:27
sanomari
sanomari approved these changes Oct 11, 2021
View reviewed changes
Copy link
Contributor

@sanomari sanomari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GHA/AZP(MinGW含む)/Appveyorでバージョンが揃ってるのを見ました。
ダウンロードファイル名の変更も対応すると分かりやすくなると思います。

k-takata
k-takata reviewed Oct 11, 2021
View reviewed changes
ci/azure-pipelines/template.job.build-on-msys2.yml
fetchDepth: 5 # the depth of commits to ask Git to fetch; defaults to no limit

- bash: |
BUILD_VERSION=`git log --oneline --no-merges | wc -l`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コミット数のカウントには git rev-list --count --no-merges @ が使えます。（githash.bat についても同様）

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

git rev-list --count --no-merges @ が使えます。

これだとwc -lやfind /C " "と併用せずに数えられますね。
ありがとうございます。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ありがとうございます。これならMinGWビルドのコミットカウントを別にする必要もなさそうです。
これはこれとして別PRで対応しようと思います。

@berryzplus
Copy link
Contributor Author

レビューありがとうございます。マージしちゃいます。
#1738 (comment) の件は別PRで対応します。

@berryzplus berryzplus merged commit a671199 into sakura-editor:master Oct 11, 2021
@berryzplus berryzplus deleted the feature/refactoring_version_info branch October 11, 2021 15:20
@berryzplus berryzplus mentioned this pull request Oct 11, 2021
Merged
@m-tmatma m-tmatma mentioned this pull request Dec 3, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@k-takata k-takata k-takata left review comments

@sanomari sanomari sanomari approved these changes

Assignees
No one assigned
Labels
appveyor azure pipelines CI appveyor など CI 関連 【ChangeLog除外】 GitHub Actions GitHub Actions関連 🌏Internationalization 🇯🇵 🇺🇸 🇨🇳 🇹🇼 国際化対応 local build management 運営に関する話題 【ChangeLog除外】 specification change ■仕様変更
Projects
None yet
Milestone
No milestone
4 participants
@berryzplus @AppVeyorBot @k-takata @sanomari