mmdeploy icon indicating copy to clipboard operation
mmdeploy copied to clipboard

improvement(cmake): simplify build option and doc

Open tpoisonooo opened this issue 1 year ago • 1 comments

Thanks for your contribution and we appreciate it a lot. The following instructions would make your pull request more healthy and more easily receiving feedbacks. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.

Motivation

Build option is too complex for me. Now that I have specified ncnn_DIR=xx, why have I typing TARGET_BACKENDS=ncnn...

If you want to use ncnn, cmake -Dncnn_DIR=${NCNN_DIR}/build/install/lib/cmake/ncnn .. should be all you need.

Please first review https://github.com/open-mmlab/mmdeploy/pull/842

Modification

  • Remove repeated MMDEPLOY_TARGET_BACKENDS option
  • Remove redundant build option description in doc, add a cmake_option.md
  • Using g++-7 if g++ compiler not specified
  • Update doc

Checklist

  1. Pre-commit or other linting tools are used to fix the potential lint issues.
  2. The modification is covered by complete unit tests. If not, please add more unit tests to ensure the correctness.
  3. If the modification has a dependency on downstream projects of a newer version, this PR should be tested with all supported versions of downstream projects.
  4. The documentation has been modified accordingly, like docstring or example tutorials.

tpoisonooo avatar Jul 28 '22 15:07 tpoisonooo

Codecov Report

Merging #832 (9d640d0) into master (f80c90e) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 9d640d0 differs from pull request most recent head b345756. Consider uploading reports for the commit b345756 to get more accurate results

@@           Coverage Diff           @@
##           master     #832   +/-   ##
=======================================
  Coverage   54.65%   54.65%           
=======================================
  Files         280      280           
  Lines        9289     9289           
  Branches     1349     1349           
=======================================
  Hits         5077     5077           
  Misses       3858     3858           
  Partials      354      354           
Flag Coverage Δ
unittests 54.65% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Jul 28 '22 15:07 codecov[bot]

May be still need change jetsons.md to remove -DMMDEPLOY_TARGET_BACKENDS options explicitly. The same about zh-cn version.

hanrui1sensetime avatar Aug 08 '22 11:08 hanrui1sensetime

May update the building way of the dockerfile as well.

AllentDan avatar Aug 11 '22 09:08 AllentDan

May update the building way of the dockerfile as well.

done.

tpoisonooo avatar Aug 15 '22 11:08 tpoisonooo

Please fix CircleCI first, and then LGTM.

done.

tpoisonooo avatar Aug 16 '22 01:08 tpoisonooo