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.

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

プロジェクト設定内の重複した記述をまとめたい #1775

Merged

Conversation

berryzplus
Copy link
Contributor

PR の目的

vcxproj内の重複した記述をまとめて、読みやすくします。

カテゴリ

  • ビルド関連
  • その他の問題

※正確には、プロジェクトファイルの「リファクタリング」ですが、
アプリ機能を触らないので「その他」としました。

PR の背景

サクラエディタの「プロジェクト設定」は、sakura.vcxprojに書いています。

vcxproj は MSの汎用ビルドツールである MsBuild の設定ファイルです。
「ビルドに汎用ツールを使う」ということは、フォーマットも汎用的なものになっています。
C#VB.NETと同じ、XMLフォーマットになっています。

XMLフォーマットってことは「人間に読めるフォーマット」です。

自動生成ではないんだから、
もっと「手作り仕様」にしようぜ?

#1730 の対応はそういう経緯と理解してます。

このPRは「手作り仕様」をもう一歩進めようとする試みです。

PR のメリット

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

  • 「まとめない方が分かりやすい」と考える人がいるかも知れません。

仕様・動作説明

プロジェクト設定内の重複した記述をまとめます。

変更前イメージ

  • Win32 DebugではMFCを使いません。
  • Win32 ReleaseではMFCを使いません。
  • x64 DebugではMFCを使いません。
  • x64 ReleaseではMFCを使いません。

変更後イメージ

  • MFCを使いません。

何が違うか?

  • 変更前は、ビルド設定4つを見て始めて「全部同じだね」が分かります。
  • 変更後は、ビルド設定1つを見れば「共通設定なのね」が分かります。

PR の影響範囲

アプリの仕様・機能には影響しないと思われます。

MSVCビルドに影響する変更です。
ビルド設定の重複記述をまとめる修正なので、
ミスった場合はビルド設定が変わってしまう可能性があります。
※目視チェックできそうな分量なので問題はないと思いますが。

テスト内容

CIでビルドが通っていれば確認は十分と思います。

テスト1

手順

関連 issue, PR

参考資料

@sonarcloud
Copy link

sonarcloud bot commented Jan 25, 2022

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
No Duplication information No Duplication information

@AppVeyorBot
Copy link

Copy link
Contributor

@beru beru left a comment

Choose a reason for hiding this comment

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

統一しても問題無い設定だと思います。

Visual Studioでプロジェクトのプロパティで変更した時に従来は個別に切り替わっていたのがそうではなくなるので、違和感を感じる人がいるかもしれませんが、vcxproj を見れば理解出来ると思います。

あと vcxproj を手書きしないで CMakeLists.txt に任せるのはどうだろうかと思いましたが、クロスプラットフォーム対応していないしメリット薄そうですね。

@berryzplus
Copy link
Contributor Author

レビューありがとうございます。マージしちゃいます。

@berryzplus berryzplus merged commit df60714 into sakura-editor:master Jan 26, 2022
@berryzplus berryzplus deleted the feature/refactoring_vcxprojects branch January 26, 2022 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants