새로 입사한 개발자가 프로젝트에 기여하는 방법 한 가지

프로젝트 전체의 경고를 감소 시켜 팀에 기여하자!

이종립 게시 날짜: 2020.03.09.

코딩 스타일 가이드를 읽다가 작성한 Pull Request

안녕하세요. LDD팀 이종립입니다.

오늘은 제가 입사한 지 일주일쯤 지났을 무렵, 즉 2019년 11월에 있었던 이야기를 해보려 합니다.

팀 문서 공간에 코딩 스타일 가이드가 있기에 흥미롭게 읽다가 중괄호를 optional 하게 사용하지 않는다는 항목을 읽게 되었습니다.

이 규칙이 잘 지켜지고 있는지 갑자기 궁금해졌던 저는 맥북 터미널을 열고 검색을 해 보았습니다.

 # 여러분도 해보세요
find . -name '*.java' \
  | xargs egrep '^\s*if[^\{]*\s*$' --no-filename

(find에 -exec를 주지 않고 파이프로 연결한 이유는 속도 때문입니다)

앗 5개 밖에 없잖아?!

문서를 다시 읽어보면 중괄호 생략을 허용하는 경우는 "예외 케이스에 대한 종결인 경우" 뿐입니다.

따라서 검색 결과에 나온 5줄의 코드는 모두 팀 코드 컨벤션을 위반하는 것이었습니다.

신규 입사자가 기여하기 적당한 문제 같았으므로 간단히 작업해 PR을 제출하였습니다.

PR은 곧 머지되었습니다.

프로젝트 전체에 경고가 1,823개?

저희 LDD 팀은 IntelliJ IDEA에 CheckStyle-IDEA 플러그인 설정을 권장합니다.

팀의 규약을 CheckStyle 설정으로 표현한 kurly_checks.xml라는 파일도 공유하고 있습니다.

그래서 저도 CheckStyle을 설정한 다음 프로젝트 전체를 대상으로 돌려보았습니다.

PR을 더 보내면 프로젝트에도 기여할 뿐 아니라, 팀의 코드 베이스를 파악하는 데에도 도움 될 것이라고 생각했기 때문입니다.

그런데…

경고가 1,823개?!

시간이 날 때마다 조금씩 없애가는 수밖에 없겠다고 여기면서, 문제의 코드를 다양한 방향으로 활용할 수 있겠다는 생각이 들었습니다.

기계적으로 제거할 것인가? 팀 성장의 기회로 삼을 것인가?

경고 코드를 없애는 가장 쉬운 방법은 프로젝트 전체에 포매터를 돌려버리는 것입니다.

그러나 굳이 그렇게 하지 않기로 했습니다.

그 이유는 다음과 같습니다.

팀의 Github 활용을 위한 학습 자료

  • 당시 팀은 Github의 Pull Request의 활용을 적극적으로 도입하고 있었던 시기였습니다.
  • PR에서 코드 리뷰, 머지로 자연스럽게 이어지는 사이클을 학습하기에 좋은 자료라고 생각했습니다.
    • 저희 팀이 코드 리뷰를 안착시킨 과정은 다음에 올려 보도록 하겠습니다 :)

터미널에 익숙하지 않은 동료를 위한 학습 자료

  • git 명령에 익숙하지 않은 동료를 위해 commit 방법과 commit 단위, 작업 브랜치 등에 대한 좋은 설명 자료가 되었습니다.
  • 터미널에 관심이 많은 동료분을 위한 학습 자료로 활용하였습니다.

코드 컨벤션의 중요성을 강조할 기회

  • 팀의 코드 컨벤션은 약속입니다.
  • 구현만이 중요한 작업이 아니라 컨벤션을 지키기 위한 작업도 의미가 있다는 것을 강조하고 싶었습니다.

시간상의 여유가 있었음

  • 신규 입사자라 시간에도 여유가 있었습니다(…)
  • 그냥 포매터를 돌려버리면 재미가 없습니다.

시지포스의 형벌

경고는 없애도 없애도 다시 나타나게 됩니다.

그러나 저는 보이스카웃 규칙에 따라, 오늘 발견한 경고를 없앨 것입니다.

The Boy Scout Rule : "Always leave the campground cleaner than you found it."

보이 스카웃 규칙 : 언제나 처음 왔을 때보다 깨끗하게 해놓고 캠프장을 떠날 것.

발견할 때마다 조금씩 청소한다면 코드 베이스는 점진적으로 깨끗하게 되겠죠?

if, for 문에 붙은 괄호에 공백을 주자

가급적이면 문제 소지가 적은 것부터 작업하고 싶습니다.

다음 규칙이 눈에 들어옵니다.

이 규칙이라면 작업을 해도 뭔가 잘못되거나 심각한 오류가 발생할 것 같지는 않네요.

이번에는 제가 egrep을 대신해 즐겨 사용하는 ag를 써서 for ( 여야 하는 코드가 for(로 잘못 작성된 것이 없는지 검색해 보았습니다.

wc -l로 세어보니 49건이 나왔습니다. 이 정도면 할만합니다.

이런 작업은 sed를 쓰면 쉽게 끝나겠죠.

검색 명령어는 이미 만들어뒀으니, 화살표를 위로 올린 다음 바로 뒤에 파이프를 넣어주고 sed로 연결해서 간단하게 끝냈습니다.

KEYWORDS="(if|for|while|try)"
ag "$KEYWORDS\(" -l \
  | xargs sed -i ''  -E "s/$KEYWORDS\(/\1 (/"

git diff로 결과를 보니 10개의 대상 파일에 문제없이 잘 적용되었습니다.

PR도 올려 보았습니다. 아주 간단한 작업이기 때문에 Change request 없이 곧바로 Approved를 받아낼 수 있었습니다.

(내용 이해에 불필요한 항목은 삭제한 이미지입니다)

대문자로 시작하는 lambda 변수 이름을 소문자로 시작하게 바꾸자

이 작업은 다음 명령으로 수행했습니다.

ag '\(([A-Z]\w*)\s?\-\>\s*\1' -l \
  | xargs gsed -i.orig -e '/\.filter/ s,(\([A-Z]\),(\L\1,; s,-> \([A-Z]\),-> \L\1,'

(sed의 -i.orig는 .orig 확장자를 가진 백업 파일을 생성하라는 의미입니다)

실행한 다음 git diff를 보니 다음과 같이 대문자가 소문자로 잘 변환되었습니다.

테스트 코드가 잘 돌아가는 것을 확인하고, 생성된 백업 파일을 제외한 변경 파일을 add, commit 한 다음 PR을 올렸습니다.

물론 PR 내용도 열심히 작성했습니다.

화살표 연산자 좌우에 스페이스를 1개 추가하자

이 작업은 다음과 같이 수행했습니다.

find . -name '*.java' \
  | xargs ag '\-\>(?=\S)|(?<=\S)\-\>' -l \
  | xargs sed -i.orig -E "s,([^ ])->,\1 ->,; s,->([^ ]),-> \1,"

이 작업은 꽤 재미있었는데 오래간만에 전후방 탐색을 사용했기 때문입니다.

프로젝트 전체에서 탭 문자를 sed로 2 spaces로 교체하자

다음 명령으로 프로젝트의 모든 java 파일에 있는 탭 문자를 2개의 스페이스로 교체했습니다.

find . -name '*.java' \
  | xargs ag '\t' -l \
  | xargs sed -E -i '' "s/[[:cntrl:]]/  /g"

물론 이 작업도 PR로 올리고 곧 머지되었습니다.

여는 중괄호 앞에 스페이스를 1개 추가하자

이 작업은 위의 작업들에 비하면 매우 쉬운 작업이었습니다.

ag '\)\{' -l | xargs sed -i.orig 's/){/) {/'

결과

위에 소개한 작업 외에도 다양한 작업이 있었습니다. 분량의 문제로 모두 소개할 수 없어 이만 줄이도록 하겠습니다.

위와 같은 작업을 거쳐 해당 프로젝트는 500개 이상의 경고를 제거할 수 있었습니다.

그리고 새로 시작하는 프로젝트는 거의 언제나 1개 이하의 경고를 유지하고 있습니다.

물론 이 작업의 직접적인 결과는 아니지만, 이 작업을 포함한 팀원들의 다양한 노력의 결과로 저희 팀은 컨벤션을 중요하게 고려하여 코딩을 하고, git commit 메시지를 신중하게 작성하고, Pull Request를 올리고, 코드 리뷰를 거쳐 머지에 도달하는 프로세스가 자리 잡게 되었습니다.

다음에는 팀에 코드 리뷰가 안착하게 된 과정과 그 전략에 대해 자세히 써볼까 합니다.

읽어주셔서 감사합니다.