최근에 이직한후 과도한 코드리뷰 때문에 스트레스를 좀 받고있습니다, 제가 예민한건지 좀 과도한건지 객관적인 조언 좀 부탁드립니다
몇몇 케이스들이 있었지만 기억나는거 위주로 써보겠습니다
참고로 여기서 말하는 신입은 팀에 신입이지 경력이 신입은 아닙니다
다들 비슷한 연차이고 차이 젤 많이 나는사람이 2년이하로 차이납니다
1. 10줄짜리 함수 2개의 nested for loop 그안에 if문두개
리뷰 : for loop이 두개라 읽기가 너무 힘드니깐 안에 포문 한개 + if문2개 다른 함수로 빼는 refactor해라
제 의견 : 10줄 짜리 함수를 못알아볼 정도면 시니어 자격이나 있는건가?
-> 제가 for loop두개짜리는 읽기에 크게 어렵지않다고 답글남기고 그냥 넘어갔습니다
2. 자잘한 변수명 지적
config = get_alert_config()
do_someting(config_name = config.name)
이게 대략적인 코드인데 alert에 대한 config에 name이라는 필드가 있습니다
리뷰 : config_name을 alert_name으로 변경해라
의견 : config_name 이나 alert_name이나 그게 그거 아닌가? 헷갈린다면 alert_config_name이 오히려 더 나은거 아닌가?
-> 그냥 해달라는데로 변경해줬습니다
3. 프로덕션에서만 실행되고 개발환경에서는 raise exception 되는 함수가 있습니다
kube_client.get_name() #간단히 개발환경은 쿠베를 쓰지않으니 exception이 생겨서 현재 코드에서
if is_kube():
kube_client.get_name()
이런식으로 개발환경에서는 예외처리가 되게끔 최근에 추가했고 팀원이 다 있는곳에서 회의를 통해서 결정한 사항입니다
다른 곳에서 똑같은 코드를 발견해서 똑같이 if is_kube()를 추가해줬습니다
리뷰 : 우리 이미 이런 똑같은 이슈가 있었자나 (URL코드링크) 왜 새로운 문제가 생겼는지 알려달라
의견 : 이미 똑같은 이슈가 있으니 여기도 똑같이 처리하는건데....?? 새로운 문제가 뭔지 PR Summary에 적어둠, 근데 Summary읽고도 context가 부족하니 리뷰어가 이해할수 있도록 summary다시 적어달라고함.....
-> 어떻게summary를 업데이트할지 몰라서, 똑같은 문제가 있었으니 여기서도 문제가 생기는거고 똑같이 처리하는것이다 라고 답글
4. 새로운 맴버들을 검증/테스트?
하나에 서비스에 memory leak이 있어서 저보고 해결하라고 하더군요, 뭐 대략적인 가이드라인은 줬습니다 (네...제가 첨에 제시한 방법이였어요)
저희는 서비스를 32개의 서버에서 운영하는데 당연히 2~4개정도에만 테스트를 진행하면서 퍼포먼스가 떨어지지않는지 memory leak이 해결됐는지 모니터링하면서 최종적으로 32개 모든 서버에 적용할 생각이였습니다
갑자기 기존맴버들이 저보고 회의좀 잡아달라고해서 잡으니깐 새로운 맴버들끼리 토론을 해보랍니다.....
이런경우는 어떻게 접근하는지 좋을지 뭐 그래서 아 자기가 회의를 잡으라고 해놓고 자기는 뭐 아무것도 안하겠다는건가 생각하고있었는데
뭐 다른 팀맴버가 자기 생각을 쭉얘기하길래 저도 제 생각을 얘기하니깐 그래 제대로 가고있어 좀더 생각해바
이런식으로 아침 주니어 다루듯? ㅋㅋㅋㅋ 좀 불쾌한 느낌이 많이들었습니다
참고로 기존맴버 두명은 저랑 연차가 똑같고 정확히 따지면 저보다 몇개월 짧습니다
물론 지금 회사에서 신입으로 들어와서 이때까지 쭉있었던 사람들이라 이 회사에대한 경험을 휠씬 더 많습니다
5. 새로운 맴버는 코드리뷰 하지마라
제가 개발하다가 코드 구성이 좀 잘못되어있는게 있고 circular imort문제가 있어서 refactor을 진행했습니다
어차피 refactor니깐 다른 새로운맴버가 보고 크게 이상없고 unittest 다 통과되니깐 approve해줬습니다
이걸로 거의 취조를 당했습니다 이걸 왜했냐 안하고 다른 방법은 없었냐 그리고 제코드 approve해준 신입팀맴버에게는 앞으로 제코드 review하지말라고 말했더군요
6. 신입맴버에게만 적용되는 높은 잣대
매니저가 우리팀의 서비스를 점점더 많은 이용자가 사용하기 시작했고 우리팀의 버짓이 2025년도에 3배가 늘었습니다
그러니 높은 잣대를 모든 엔지니어에게 요구할것이고 따라줬으면 좋겠다고 했습니다
물론 동의합니다 높은 잣대로 코드를 리뷰하고 좋은 서비스 만들어 가는거 중요합니다
근데 높은 잣대가 신입맴버한데만 적용되는거같습니다
높은 잣대라는건 위에서 부터 밑으로 내려온다고 생각하는데 기존 시니어 맴버들은 코드에 별 짓을 다해놓고 자기들끼리 approve하고
신입한데는 변수명까지 따지는 상황이죠
그리고 memory leak을 어떻게 해결하면 좋겠냐는 회의에서 기존 시니어 맴버가 memory leak을 잡는게 중요한게 아니라 뻣지않을만큼 memory leak을 통제하는것도 방법이 될수있다 (??) 뭔 개소리 ㅋㅋㅋ
뭐 기존에 있는 개발자도 새로온 개발자가 마음에 안드는 부분이 있겠지만
신나게 코드쓰고나면 또 어떤 말도 안되는것으로 시비걸고 넘어질지 스트레스를 좀 받습니다
조언좀 부탁드립니다
내용상으로는 컨벤션 문서가 제대로 되어 있다면 리뷰는 문제 없어 보입니다.
nested for문이나 nested if 같은 경우에 특정 상황에만 예외를 두고 일괄로 금지 시키는게 컨벤션적으로는 좋을 거에요.
근데 어찌됐든 컨벤션은 정하기 나름이고 문서 등이 없는 상태에서 리뷰만으로 그때 그때 저런 식으로 하는 거면 문서 작성을 제안해보시면 어떨까요? 명확히 컨벤션이 정의되어 있는것이 좋을테고 (네이밍 규칙 포함) 초반에 신규 멤버라면 규칙을 따라가려고 노력하면서 1on1을 통해 불편한 사항 등을 제안해보셔도 좋을 듯 합니다.
쓰신 글로는 서로에 대한 존중이 부족해 보입니다. 글쓴 분은 불쾌한 경험으로 인해 기존 인원들을 `개소리` 등의 단어로 무시하는 듯한 느낌이 들고 기존 인원들은 명확한 규칙없이 텃새? 를 부리는 느낌이 드네요.
일단 컨벤션, 네이밍 규칙 등이 명시되어 있지 않다면 체계적인 환경은 아닌거 같은데 어느정도 규모의 팀인지 궁금합니다.
(물론 로직에 문제가 있으면 당연히 고쳐야 합니다.)
대기업이어도 멀쩡한 가이드가 있기 힘듭니다. 이 경우, 빅테크 회사 가이드를 따르고 이마저도 강제가 아니라고 생각 됩니다. 강제를 하려면 서브밋 전에 툴이 막는 정도는 되야 의미가 있다고 생각합니다.
기존 코드에 대해서 리뷰를 하는 것은 힘듭니다. 새로 짜거나 수정 되는 코드부터 리뷰하게 되죠. 이건 당연한 거 같아요...
내가 똥을 싸면 남이 치워야합니다. 남이 똥을 싸면 내가 치워야하죠.
Gobi 님이 의견남기신 것처럼 코드 컨벤션은 명확히 문서화해두셔야 합니다.
또 기존에 처리된 이슈를 refactoring 하는 건 그 시점에 같이 처리되었어야하나 급해서 처리가 어려웠다면 당연히 해야하는 것이구요. 기존 처리된 티켓을 reference로 적어두시면 될것 같습니다.
아마도 code review 나 refactoring을 제대로 해본적이 없어서 그런것으로 보입니다.
이건 책이나 강의로 해결하긴 쉽지 않고 이런 경험이 많은 시니어? 멘토?? 이런 사람이 필요한 영역이긴 합니다.
좀 더 팀원들과 대화를 하실 필요가 있을 것 같아요.
1번 같은 경우에도 저 같은 경우는 꼭 바꿔 달라고 합니다. 뭐 각기 다른 이유가 있을 수 있겠지만..시니어냐 아니냐의 문제는 아닌 것 같구요. 왜 힘든지, 제안받은 방법이 좋다고 생각한 이유는 무엇인지 대화를 하면서 풀어가는 게 좋지 않나 생각이 드네요.
2번 같은 경우도, 왜 바꿔야 하는지, 변수명 에 대한 규칙이 있어서 그런 것인지. 서로의 생각을 맞춰가는 게 좋을 것 같다는 생각이 드네요.
기존에 있는 개발자가 새로 합류한 개발자가 마음에 안드는 부분이 있다?그럼 뽑지 말았어야죠. 같이 일하는 팀원이 되었다면 서로서로 많은 대화를 하면서 문화를 만들어가는 것이 좋다고 봅니다.
저도 코드리뷰는 강제되어야 한다고 생각하구요. 사실 변수명이나 컨벤션으로 리뷰를 남기자니, 힘들죠..그런 것보다는 개인적으로는 요구사항에 맞게, 어떤 것을 개발했는지, 테스트 코드에 명시가 되어 있는지 등을 보는 것이 더 좋은 상황인 것 같네요.
의견을 그대로 듣고 대화해야 합니다.
그것이 합리적이면 바꾸는거고 아니면 반론을 내는데 여기서 더 나가서 내가 신규직원이라 더 그런다, 연차도 비슷한데 그런다까지 가면
스트레트가 한도초과되더라구요.
물론 신규직원얘기는 외부에서 들어온 거지만 그래도 머리에서 빼내면 한결 편해질겁니다. 회사가 뭐라고 이런 내용으로 애를 쓰나.. 싶을수도요.
평균적으로 사회성이 부족한 프로그래머들 세상에서
공정한 대우는 있더라도 금방 사라졌다가 생겼다가 하더라구요.
아무쪼록 우리 모두 회사에서 스트레스 덜 받으시면 좋겠습니다.
항목별로 저의 편견을 그득그득 담아서 썼습니다.
본문만 보고 이래저래 판단하면 보통 틀리기에
정답을 주장한다기보다 제 경험을 공유한다고 봐주세요.
- 1번. 리뷰는 사고방지가 주 목적이고, 조금 더 나가면 코딩문화를 잡는 겁니다. 코딩문화라고 치면 일리 있다고 봅니다. 1,000줄 리뷰가 흔하진 않지만, 복잡한 로직 리뷰하는데, 중간에 10줄짜리 이중 for문이 있으면 10줄치 이상의 스트레스가 됩니다. 이해는 하지만 후루룩 보고 넘길수는 없는 시간이죠. 물론 이건 정답이 아닙니다.
- 2번. config가 한두개가 아니라면 "alert"이 있을때 의미가 더 명확해보입니다. 저라면 alert_config.name에 한표에요. alert_config = get_alert_config()
- 3번. 이건 소통의 문제이며, 같은 이슈가 같은 곳에서 발생하지 않았음을 명시하면 되는 내용입니다. 대응 잘 하신 것 같습니다. 그 이상을 바란다면 저는 "알겠습니다" 하고 그냥 다른 일 합니다. 생각을 멈추고 다른 일 하는게 효율적입니다. 상대방은 그렇게 얘기하고 다른일 하고 있을거에요. 까먹을수도 있습니다.
- 4번. 저 개인적으로 회의를 할때 안건 없이 시작하는건 정말 초보적인 소통문화라고 생각합니다.
미리 양질의 정보를 준비도 못해서 답도 안나와서 2차회의 잡혀요.
안건이 없으면 이런저런 생각나는 안건 다 나오기도 해요. 그러면 그날 일할 시간이 부족합니다.
이건 확실히 안 좋은 문화긴 해요. 이런 게 반복되면 안되지만, 또 회사문화가 이쪽으로 잡혀 있으면 어쩔수 없더라구요.
- 5번. test code역할에 대해 논의가 필요해 보입니다. 커버리지가 높아서 사고방지 역할이 되는지, 일부 기능만 테스트 가능해서 이번에 나간게 문제가 된건지. 아니면 변경한 코드에 대한 테스트코드가 없었던 건지.
- 6번. 기존 시니어 업무량이 많다면 그럴 수 있다고 봅니다. memory leak 통제도 일리가 있습니다.
팀 능력이 안되면 사고라도 막아야하지요. 빅테크기업도 한번씩은 필요한 대응을 못할 때가 있습니다. 일정을 지키기 위해, 서비스 유지를 위해. 갑자기 중요한 인물이 그만둬서.
예전 사례로 메모리가 차오르는 서버를 주기적으로 껐다 키는 대응도 있었는데, 시간을 벌어두고 차근차근 디버깅하는 개념이었어요.