코드 리뷰 봇은 "이 검사는 grep 으로 이렇게 하세요" 같은 명령과 정규식을 자주 제안한다. 제안이 그럴듯해 보여 그대로 적용하면, 실제 파일에는 하나도 매치되지 않는 검사가 생길 수 있다. 이런 검사는 실패하지 않고 항상 빈 결과를 내므로 통과한 것처럼 보인다. 이 글은 봇 제안을 적용하기 전에 어떤 확인을 하는지 정리한다. 번호가 붙은 문서를 찾는...
코드 리뷰 봇은 "이 검사는 grep 으로 이렇게 하세요" 같은 명령과 정규식을 자주 제안한다.
제안이 그럴듯해 보여 그대로 적용하면, 실제 파일에는 하나도 매치되지 않는 검사가 생길 수 있다.
이런 검사는 실패하지 않고 항상 빈 결과를 내므로 통과한 것처럼 보인다.
이 글은 봇 제안을 적용하기 전에 어떤 확인을 하는지 정리한다.
번호가 붙은 문서를 찾는 검사를 예로 든다.
저장소에 030-slug.md, 031-other.md 처럼 번호 뒤에 slug 가 붙은 파일이 있다.
봇이 이 파일들을 찾는 패턴으로 [0-9]+\.md$ 를 제안했다고 하자.
$ printf '030-slug.md\n031-other.md\n' | grep -cE '[0-9]+\.md$'
0
$ printf '030-slug.md\n031-other.md\n' | grep -cE '[0-9]{3}-.*\.md$'
2제안된 패턴은 숫자 바로 뒤에 .md 가 와야 해서 030-slug.md 에 매치되지 않는다.
이 패턴으로 "번호가 이미 쓰였는지" 검사하면 어떤 번호든 쓰이지 않았다고 판정한다.
스크립트는 오류 없이 끝나므로 검사가 죽었다는 사실이 어디에도 드러나지 않는다.
번호 충돌을 막으려고 만든 검사라면, 충돌이 나는 순간에도 통과하는 셈이다.
패턴뿐 아니라 제안의 이유도 틀릴 수 있다.
봇이 diff a b || echo changed 같은 줄을 두고 "diff 는 차이가 있으면 0 이 아닌 종료 코드를 내므로 set -e 스크립트가 중단된다" 고 지적한다고 하자.
이미 || 로 비정상 종료를 처리하고 있어서 그 지적은 성립하지 않는다.
$ echo a > a; echo b > b
$ bash -c 'set -e; diff a b >/dev/null || echo handled; echo continued'
handled
continued|| 의 왼쪽 명령이 실패해도 set -e 는 스크립트를 중단하지 않는다.
이유를 확인하지 않고 "안전하게" 고치면 불필요한 변경이 들어간다.
봇 제안을 수용하기 전에 두 가지를 한 번씩 돌려 본다.
grep 이나 정규식을 현재 diff 와 실제 파일 이름에 한 번 실행해 기대한 개수만큼 매치되는지 본다.둘 중 하나라도 어긋나면 보정해서 적용하거나, 이유를 적고 기각한다.
위 예에서는 [0-9]{3}-.*\.md$ 로 보정해 두 파일 모두 매치되는 것을 확인한 뒤 적용한다.
검사를 추가할 때는 지금 저장소에서 매치되는 개수를 기대값으로 함께 기록해 두면 이후에 패턴이 죽었을 때 알아챌 수 있다.