search

Qulacs-Osaka / qulacs-osaka

Development branch of qulacs at Osaka Univ
MIT License
13 stars 6 forks source link

default USE_TEST=No #339

Closed KowerKoint closed 2 years ago

KowerKoint commented 2 years ago

close #338 CMakeLists.txtのUSE_TESTのデフォルトをNoにして、代わりにビルド実行時にオプションでUSE_TEST=Yesをつけるように変更しました

↓WindowsのCIをdispatchしたやつ https://github.com/Qulacs-Osaka/qulacs-osaka/actions/runs/2485651160

forest1040 commented 2 years ago

LGTMです！

ikanago commented 2 years ago

devcontainer を使う人は USE_TEST=Yes になってるほうが嬉しいと思うので，Dockerfile の環境変数に加えてしまっていいと思いますがどうでしょうか?

codecov-commenter commented 2 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (dev@81caabf). Click here to learn what that means. The diff coverage is n/a.

@@          Coverage Diff           @@
##             dev     #339   +/-   ##
======================================
  Coverage       ?   70.92%           
======================================
  Files          ?       84           
  Lines          ?     5971           
  Branches       ?        0           
======================================
  Hits           ?     4235           
  Misses         ?     1736           
  Partials       ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 81caabf...3c13051. Read the comment docs.

KowerKoint commented 2 years ago

devcontainer を使う人は USE_TEST=Yes になってるほうが嬉しいと思うので，Dockerfile の環境変数に加えてしまっていいと思いますがどうでしょうか?

確かにそうですね 加える方針にします。

KowerKoint commented 2 years ago

https://github.com/Qulacs-Osaka/qulacs-osaka/pull/343 含めました

KowerKoint commented 2 years ago

ただ、USE_TEST=Yesの状態でも普通にコンパイルする分にはtestコードはコンパイルしないので、分岐が増える割には有効性が低いと思うのですがどうでしょうか？

もともとこれを直そうと思った目的が #338 で言及してる、「デベロッパーじゃなくてもカバレッジを巻き込んでビルドしてしまったせいでリンクを要求してしまった」というもので、どうせそこで差をつけるならUSE_TESTをNoにして必要のないGoogle Testのインストールもカットしようと思ってました