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

doxygen の設定ファイルと実行用のバッチファイルを追加 #565

Merged
merged 10 commits into from
Nov 22, 2018

Conversation

m-tmatma
Copy link
Member

doxygen の設定ファイルと実行用のバッチファイルを追加

最小のコミットで
doxygen -g doxygen.conf
の生成ファイルを追加して、差分のコミットを積んでいます。

@m-tmatma m-tmatma added the document ドキュメント label Oct 19, 2018
@m-tmatma m-tmatma added this to the next release milestone Oct 19, 2018
@berryzplus
Copy link
Contributor

あとでgdgdするのもアレなので気になったこと今書いておきますね。

最小のコミットで
doxygen -g doxygen.conf
の生成ファイルを追加して、差分のコミットを積んでいます。

  1. 記載のコマンドはデフォルト設定を出力するコマンドと理解しました。
    で、 b701501 がその出力内容。
    これの生成時間がどれくらいになるのか分かりませんが、自動生成ファイルを登録することの是非がどうなのか気になりました。バッチを登録してるので、その気になればデフォルト設定を生成してその場でパッチ当ててもいいはずだと思ったわけです。appveyorにはgitがインストールされてるのでpatchも当然入っているだろう、という発想のもと、裏はとってないんですが。
  2. バッチファイルの処理内容がやや気になりました。
    構造的には3行目の環境変数がdoxygenの実行(設定?)に必要だから
    1行目のgithash.batを走らせているのかな?と思うんですけど、
    これってコメントなしで誰でも分かる内容なんだっけ・・・と思ったわけです。

きっとこのあとこのbatをappyorに組み込んだりドキュメント書いたりって作業が続くと思ってます。

完全に「お任せ」になってしまうのでとりあえず突っ込みだけ入れてみました。
PRの判断はLGTMのつもりですが、先に @file の方をみないといかんので一旦保留とします。

berryzplus
berryzplus previously approved these changes Oct 21, 2018
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です。

conversationに書いたのは今後の課題ということで後追いでよいと思います。
実質20行足らずの変更が2512行って、ちょっとビックリしますけどね。
問題ないと思います。

@m-tmatma
Copy link
Member Author

誤: 最小のコミットで
正: 最初のコミットで

でした。

@m-tmatma
Copy link
Member Author

これの生成時間がどれくらいになるのか分かりませんが、自動生成ファイルを登録することの是非がどうなのか気になりました。バッチを登録してるので、その気になればデフォルト設定を生成してその場でパッチ当ててもいいはずだと思ったわけです。appveyorにはgitがインストールされてるのでpatchも当然入っているだろう、という発想のもと、裏はとってないんですが。

それだと設定ファイルのメンテが大変だろうと思います。
ただ何かいい運用方法があれば、issue or PR 作成お願いします。

@m-tmatma
Copy link
Member Author

2\. バッチファイルの処理内容がやや気になりました。
    構造的には3行目の環境変数がdoxygenの実行(設定?)に必要だから
    1行目のgithash.batを走らせているのかな?と思うんですけど、
    これってコメントなしで誰でも分かる内容なんだっけ・・・と思ったわけです。

そういう場合は、approve せずに、これじゃわからん、説明してといってほしいです。
そしてバグってました。

doxygen.conf の PROJECT_NUMBER の設定で環境変数で上書きするためにやっている
つもりです。PROJECT_NUMBER の値を設定する必要があるのですが、
設定する値が逆になっています。

また、正しく設定しても期待通り動いていません。
コマンドラインから手動で環境変数を設定したときに動いていて
バッチファイルと組み合わせたときに正しく動かない状態になっているみたいです。

@m-tmatma
Copy link
Member Author

merge した master を元に rebase しました。
上記の問題は再確認します。

@berryzplus
Copy link
Contributor

そしてバグってました。

なにいっ!ww

説明を言葉通りに受け止めるとバッチの3行目がおかしい気がします。
コミットIDにプロジェクトナンバーをセットしとります。
プロジェクトナンバーが環境変数として必要なら式が逆になるはず・・・

@m-tmatma m-tmatma force-pushed the feature/doxygen-conf branch from d198314 to 22b3206 Compare October 29, 2018 11:59
@m-tmatma
Copy link
Member Author

そしてバグってました。

修正しました。

@m-tmatma m-tmatma force-pushed the feature/doxygen-conf branch from 5462bfd to 05b9351 Compare November 6, 2018 21:59
@m-tmatma
Copy link
Member Author

m-tmatma commented Nov 7, 2018

doxygen のドキュメントを appveyor で生成して、成果物に含めるようにしました。

@m-tmatma m-tmatma force-pushed the feature/doxygen-conf branch from ded3a83 to 0b8e4ab Compare November 8, 2018 13:15
@m-tmatma
Copy link
Member Author

m-tmatma commented Nov 8, 2018

rebase しました。
レビューお願いします。

@m-tmatma m-tmatma force-pushed the feature/doxygen-conf branch from 0b8e4ab to 5ef0b14 Compare November 11, 2018 05:48
@m-tmatma m-tmatma force-pushed the feature/doxygen-conf branch from 5ef0b14 to a3d8a9f Compare November 18, 2018 12:26
@m-tmatma
Copy link
Member Author

doxygen も実行に時間がかかるので travis ci で実行するようにした方がいいかも。
#432
で、cppcheck を対応した後に検討する。

Copy link
Contributor

@ds14050 ds14050 left a comment

Choose a reason for hiding this comment

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

Release-Exe.zip のサイズが8割増しの 18MB になりました(※ただの報告です)。

生成物ができているので approve します。

@berryzplus
Copy link
Contributor

doxygenは、生成できる枠組みを作ること自体に意味があると考えています。
なので、一通り枠組みができたのなら入れてしまってよいと思います。

Release-Exe.zip のサイズが8割増しの 18MB になりました(※ただの報告です)。

中年の体重みたいな増え方ですね・・・若いころは40kgだったのにいつの間にか70kgとか。

格納先zipはまたどこかで考えるとして、問題ないと思います。

@m-tmatma
Copy link
Member Author

doxygen も実行に時間がかかるので travis ci で実行するようにした方がいいかも。
#432
で、cppcheck を対応した後に検討する。

travis-ci だと chm ファイルの生成ができないかもしれない。

@m-tmatma
Copy link
Member Author

とりあえずマージしてしまいます。

@m-tmatma m-tmatma merged commit ff76607 into sakura-editor:master Nov 22, 2018
@m-tmatma m-tmatma deleted the feature/doxygen-conf branch November 22, 2018 22:24
@berryzplus
Copy link
Contributor

travis-ci だと chm ファイルの生成ができないかもしれない。

そうなったらオプションを変えて出力形式をhtmlとかlinuxでも扱えるものにすればよいと思います。

HTML Help workshopの代替検討の話があったような。
スフィンクス?pythonを使ったツールが有力候補とか言ってたやつ・・・ → #492

@m-tmatma
Copy link
Member Author

travis-ci だと chm ファイルの生成ができないかもしれない。

そうなったらオプションを変えて出力形式をhtmlとかlinuxでも扱えるものにすればよいと思います。

chm は windows で使うには検索もできて便利なのでできれば残したい。

HTML Help workshopの代替検討の話があったような。
スフィンクス?pythonを使ったツールが有力候補とか言ってたやつ・・・ → #492

それはまた別の話ですし、ここでは doxygen の生成の話です。

sphinx の話は sakura editor 本体やマクロ等のヘルプを生成する元のデータを作って
生成できるようにするという話です。

HTML Help workshop を置き換えるものではないです。

sphinx は HTML Help workshop への入力ファイルを生成して
chm 自体はHTML Help workshop で生成します。

doxygen も chm の生成に HTML Help workshop を使っています。

@berryzplus
Copy link
Contributor

chm は windows で使うには検索もできて便利なのでできれば残したい。

残したい、了解です。

appveyorのmasterで、help生成の処理には時間がかかっていないことから、
doxygenの実行に時間がかかるとしたらchm部分じゃないんじゃないかなぁ、と思ってます。

だとしたら、htmlからchmを作るのはHTML Workshopの機能なので、
処理の分担ができないかなぁ、と思っただけです。

当面は hhc.exe が使える環境でdoxygenする、でいいと思います。

@KENCHjp KENCHjp added the CI appveyor など CI 関連 【ChangeLog除外】 label Dec 11, 2018
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019
doxygen の設定ファイルと実行用のバッチファイルを追加
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI appveyor など CI 関連 【ChangeLog除外】 document ドキュメント
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants