search

wooksong / contributon2019-nns

7 stars 6 forks source link

Apply shellcheck to all bash script files in nnstreamer and nnstreamer-example #12

Open wooksong opened 5 years ago

wooksong commented 5 years ago

There are bunch of scripts written before we enabled https://www.shellcheck.net/ in our CI system.

For the good readability or avoidance of bugs, it would be great to apply shellcheck to our existing scripts as well.

soheel commented 5 years ago

nnstreamer 및 nnstreamer_example에 있는 shellcheck 스크립트를 모두 돌려본 결과

SC2164: Use 'cd ... || exit' or 'cd ... || return' in case cd fails. SC2164: Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails. SC2086: Double quote to prevent globbing and word splitting.

이 3가지가 공통적으로 많이 나왔습니다.

이 외에도 SC2076: Don't quote right-hand side of =~, it'll match literally rather than as a regex. SC2230: which is non-standard. Use builtin 'command -v' instead. SC2059: Don't use variables in the printf format string. Use printf "..%s.." "$foo". 등과 같이 쉘 스크립트 검사 결과가 나왔습니다.

shellcheck 검사를 통해 지적받은 부분을 고쳐도 될까요?

wooksong commented 5 years ago

@soheel I have three questions:

  1. How do you check all the shell scripts in nns and nns-ex repos? Could you show me your way or code?

  2. Do you understand the meanings of the rules which shellcheck pointed out?

  3. Do you have any data about the counts of violations for each rule? If so, you need to submit that data first to the issues tab in nns, and then you better to push your PR to fix each violation separately. You can start from the most common violated rule first I guess.

THIS IS NOT AN EASY ISSUE AS YOU THOUGHT. There are many false-positives there. Please consider my questions first.

soheel commented 5 years ago

  1. 알려주신 https://www.shellcheck.net/ 로 들어가보면, 스크립트 파일을 입력하면 자동으로 검사가 되어 그렇게 테스트하였습니다.

  2. shellcheck 에서 지적한 부분들에 대해서 구글을 통해 검색해서 의미를 파악해보는 정도로 하였습니다.

  3. 오늘 내로 nns, nns-example로 shellcheck를 통해 지적한 규칙들을 쉘 스크립트 파일별로 정리하여 로드하겠습니다. 확인 후 진행하겠습니다.

확인 후 고치는 것이 의미가 없다면, 오타 수정 커밋이라도 확인하겠습니다.