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

プロジェクト構造の整理 #31

Merged
merged 6 commits into from
Jun 3, 2018
Merged

プロジェクト構造の整理 #31

merged 6 commits into from
Jun 3, 2018

Conversation

kobake
Copy link
Member

@kobake kobake commented Jun 1, 2018 •
edited
Loading

対応内容

  • _vc2017 プリフィックス削除
  • ビルドコンフィグから ANSI 版のものを削って Unicode 版のみにする。名前は Debug, Release とする。
  • sakura_lang 追加
  • HeaderMake によって自動生成されるファイル (Funccode_define.h, Funccode_enum.h) をリポジトリから除外

確認項目

  • HeaderMake, MakefileMake の起動エラーが起こらないこと
  • sakura_lang がビルドされること
  • sakura_lang が動作すること（sakura.exe と同じディレクトリに sakura_lang_en_US.dll を設置すると、設定 - 共通設定 - ウィンドウ - 言語 で「English (United States)」が選択できるようになる）

関連 Issues

@kobake kobake force-pushed the refactor-projects branch 3 times, most recently from 6b6b212 to 9103882 Compare June 1, 2018 11:15
@kobake kobake added this to the next release milestone Jun 1, 2018
@kobake kobake changed the title [WIP] プロジェクト構造の整理 プロジェクト構造の整理 Jun 1, 2018
This was referenced Jun 1, 2018
Closed
Closed
@kobake
Copy link
Member Author

kobake commented Jun 1, 2018

@berryzplus @m-tmatma
レビューいただけると助かります。よろしくお願い致します。

@kobake
Copy link
Member Author

kobake commented Jun 1, 2018

補足：一部 CI エラーが出てるのは、AppVeyor の以下エラーによるもので、これは確率で発生したりしなかったりします…。

fatal: destination path 'C:\projects\sakura' already exists and is not an empty directory.

AppVeyor 上で sakuraeditor/sakura と kobake/sakura という2か所のプロジェクトが名前で衝突していることによる問題っぽいです。AppVeyor 側の問題っぽい（？）のであとで報告入れて直してもらいたいと思っていますが、対症療法として kobake/sakura のほうの連携を #15 対応の後に解除することで、たぶん問題は起こりにくくなる気がします。

@berryzplus
Copy link
Contributor

コミット内容確認しました。対応ありがとうございます。
@m-tmatma さんにご指摘いただいていますが、該当issueはすでに閉じています。

m-tmatma
m-tmatma reviewed Jun 1, 2018
View reviewed changes
appveyor.yml Outdated
- path: sakura\Release_Unicode\sakura.pdb
- path: sakura\Debug\sakura.exe
- path: sakura\Debug\sakura.pdb
- path: sakura_lang_en_US\Debug\sakura_lang_en_US.dll
Copy link
Member

Choose a reason for hiding this comment

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

以下の順番で書いたほうが見やすいように思います。

path: sakura\Debug\sakura.exe
path: sakura\Debug\sakura.pdb
path: sakura\Release\sakura.exe
path: sakura\Release\sakura.pdb
path: sakura_lang_en_US\Debug\sakura_lang_en_US.dll
path: sakura_lang_en_US\Release\sakura_lang_en_US.dll

Copy link
Member Author

Choose a reason for hiding this comment

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

d7137e8 で対応しました！

@kobake
HeaderMake が生成するファイル (Funccode_define.h, Funccode_enum.h) をリポジトリから除外
86d411f
@kobake
sakura_lang_en_US の依存プロジェクトとして sakura を追加
0d9c105 
sakura_lang_en_US は必ず sakura の後にビルドされるようにすることで、
必ず Funccode_define.h, Funccode_enum.h が存在する状態を作る。
This was referenced Jun 2, 2018
Closed
Merged
@m-tmatma
Copy link
Member

m-tmatma commented Jun 3, 2018

なぜ何度も承認しているのですか?

berryzplus
berryzplus approved these changes Jun 3, 2018
View reviewed changes
Copy link
Contributor

@berryzplus berryzplus left a comment

Choose a reason for hiding this comment

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

LGTM

@berryzplus
Copy link
Contributor

個別に承認が必要なのかと、各コミットごとに承認飛ばしています。

@berryzplus
Copy link
Contributor

複数コミットの内容をまとめて承認する方法があればそういうやり方をしたいです。

m-tmatma
m-tmatma reviewed Jun 3, 2018
View reviewed changes
sakura/.gitignore
/Release_bcc
/Release_one
/Debug*
/Release*
Copy link
Member

@m-tmatma m-tmatma Jun 3, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

末尾の * は要らないのでは?
MakefileMake/.gitignore や HeaderMake/.gitignore では * は含まれていません。

Copy link
Member

Choose a reason for hiding this comment

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

上記以外は指摘ありません。

Copy link
Contributor

Choose a reason for hiding this comment

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

要らないとおもいます。
あっても問題ないと考えられるのでapproveと判断しました。

move Release Release_bk20180603_01
などとローカルで微妙な修正を繰り返しビルドする場合に
Release*にしておけばstage対象にならなくて便利だと思います。

Copy link
Member Author

Choose a reason for hiding this comment

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

Release_Unicode や Release_Ansi 等は明確に削除した意思はあるのですが、Release_bcc や Release_one 等は（歴史的経緯で残骸として残っているだけな気がしますが）自分としては明確に削除した意思がなかったので、念のためこれらの ignore 状態は残しておこうかと思い、* を入れておいた次第です。

現実問題として bcc, one はたぶん使われないと思いますが、将来的に x64 対応をしたとき等に出力フォルダが増える可能性はあり、そのときに楽できると良いかな、という気持ちです。

一旦今回は * は残させてください。

@berryzplus
Copy link
Contributor

いちおう、ぼくの判断としては#31と#44は一旦マージして、
何か問題があればその時に対処すればよいかな～と思っています。

m-tmatma
m-tmatma approved these changes Jun 3, 2018
View reviewed changes
Copy link
Member

@m-tmatma m-tmatma left a comment

Choose a reason for hiding this comment

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

sakura/.gitignore の指摘は直しても直さなくてもどちらでもいいです。

@m-tmatma
Copy link
Member

m-tmatma commented Jun 3, 2018

複数コミットの内容をまとめて承認する方法があればそういうやり方をしたいです。

"Files Changed" のシートに移動して、"Review Changes" を選択したらできると思います。

@berryzplus
Copy link
Contributor

ありがとうございます。真面目に分かっていなかったので助かります。

@kobake
Copy link
Member Author

kobake commented Jun 3, 2018

sakura/.gitignore 以外は問題無し、
sakura/.gitignore についても現状のままでも良いとのコメントいただいておりますので、一旦マージさせていただきます。

@kobake kobake merged commit 9196dee into master Jun 3, 2018
@kobake kobake deleted the refactor-projects branch June 3, 2018 10:17
@kobake kobake mentioned this pull request Jun 3, 2018
Closed
@m-tmatma m-tmatma mentioned this pull request Jun 3, 2018
Closed
@berryzplus berryzplus mentioned this pull request Jun 4, 2018
Closed
@KENCHjp KENCHjp added the CI appveyor など CI 関連 【ChangeLog除外】 label Dec 6, 2018
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019
@kobake
Merge pull request sakura-editor#31 from sakura-editor/refactor-projects
c2cc39e 
プロジェクト構造の整理
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@berryzplus berryzplus berryzplus approved these changes

@m-tmatma m-tmatma m-tmatma approved these changes

Assignees
No one assigned
Labels
CI appveyor など CI 関連 【ChangeLog除外】
Projects
None yet
Milestone
v2.4.0
Development

Successfully merging this pull request may close these issues.
4 participants
@kobake @berryzplus @m-tmatma @KENCHjp