LV2. 리뷰 - 이현주 기술매니저

[leehyeonj]

고생하셨습니다 ~ 리뷰 확인 부탁드립니다 ^^

1. pages 폴더

• 페이지 폴더 내에 home.js는 파일 혼자 있고 DetailPage는 폴더 내에 index.js, styled.js가 따로 있네요. 이 부분 통일 해주면 좋을 것 같습니다. 지금은 페이지가 두개 뿐이라 styled.js가 없어서 home은 폴더로 빼지 않아도 된다고 생각하셨겠지만 (아님 죄송합니당? ㅎㅎ) 파일이 많아질 수록 통일성이 중요합니다.
• 또한 home에서는 styled-component가 아니라 인라인 스타일로 되어있네요. 디테일 페이지에서도 혼재되어있구요. 이부분은 통일 시켜주는 것이 좋아요~. 컴포넌트 명을 매번 작성해야하는 번거로움이 있다면 styled-component가 아니라 scss를 쓰셔도 되고 tailwind css를 쓰셔도 됩니다.
• 클래스 명명 규칙에 대해서는 이부분 참고해보세요

2. DetailPage/index.js

• 10번째 줄 filterTodo 라는 값이 url의 params에 있는 todo값 (아이디 값이겠죠?)을 리덕스에 가져온 리스트 중에서 아이디랑 비교해서 하나만 뽑아내는 코드인걸로 보이는데요, filterTodo라는 변수명은 사실 함수명에 더 가까워요. 핸들러 명명 규칙 구글링 해서 한번 봐보세요 ~ ˙ ͜ʟ˙ 이 변수명만 보고는 todo를 filter하는 함수겠구나 라는 생각이 드는데요, 여기 들어있는 값은 하나의 todo item 이니까 const todo = ~~ 이런식으로 하는게 깔끔해 보이네요.
  그리고 그 윗줄에는 todos를 가져와서 const todo = ~~ 라고 정의를 하셨는데 todo list를 가져온거니 todoList 혹은 todos 라고 변수명을 정의하셔야 한다고 봐요.
  • styled 컴포넌트로 InnerBox라고 정의하신 컴포넌트가 있는데요, outterBox가 없는데 innerBox라고 정의하신게 어색해요. 어떻게 바꾸면 좋을까요?

3. App.js

• 라우터 path가 home 은 ‘/’ 이고 디테일은 ‘/detail/:todo’라고 되어있는데요. 일단 todo를 넘기는게 아니라 아이디를 넘기므로 :id로 하셔야 의미가 맞아요. 라우터 구조에 정답은 없지만 url만 봐도 어떤 페이지가 나올 지 예상이 되는 것이 잘 짜여진 것이라 생각이 드는데요, 그런 의미에서 /detail/:id 보다는 /todo/:id 가 더 맞지 않나 생각이 들어요 ~ 지금은 페이지가 두개 뿐이라 /:id라 하셔도 괜찮을 것 같아요.

4. 보통 함수명은 소문자로 시작합니다.

5. components/home/Header

• 헤더라고 하는 것은 통상적으로 맨 위에 붙어있는 컴포넌트를 의미해요. 여기서는

  <Headline>
          <span>My Todo List</span>
   </Headline>

  이 부분이 진짜 헤더이겠고 그 밑에는 input컴포넌트이네요. 따로 분리하시면 좋을 것 같아요.

6. pr과 issue

• pr도 올리셨고 issue도 올리시긴 했는데 이렇게 작성하시면 올리는 의미가 많이 떨어져요. 이 이슈가 어떤 이슈인지 무엇을 구현할 건지. assignee가 누구인지 적어주시면 좋겠고, pr도 마찬가지로 무엇을 구현했고 무슨 이슈를 처리하기 위한 pr이다 등등 많은 정보가 필요할 것 같아요. 그래야 협업하는 의미가 있을 것 같아요. pr템플릿이라고 구글링 해보시면 템플릿 설정하는 방법 나올거에요. 거기에 나온 내용들을 입력하는 것만으로 많이 개선이 될 것 같아요.

[Rose-Rosie-Rose]

코드 리뷰 너무 감사합니다!! 1 pages 폴더페이지 폴더 내에 home.js 파일의 통일성은 고려하지 못했던 부분인데 앞으로는 그부분까지 고려할수 있을꺼 같아서 정말 도움이 많이 된거 같습니다!! 또 style관련한 부분에서는 짧게 적은 style은 아무 생각 없이 인라인 스타일로 명시한거 같아요ㅜㅜ 매니저님께서 알려주신 방법도 많이 찾아보고 다음번에는 그러한 방식으로 구현할 수 있게 하겠습니다! 2, 3 번 부분도 조금 더 많이 찾아보고 방법을 고민해 보겠습니다!! 5 header부분은 정말 저도 많이 고민을 했는데 입력하는 부분 컴포넌트명을 어떻게 작성해야될지 고민을 하다가 header부분에 한번에 작성해 놓았는데 매니저님이 남겨주신 리뷰를 보고 이러한 방식은 좋지 않은거 같아서 더 많이 생각해보고 작성해야 되겠다고 생각이 들었습니다!! 6 pr issue는 저번에 매니저님에게 질문을 하고 나서 그 답변을 토대로 찾아보면서 level3과제를 해결하고 있었는데 pr부분은 생각지도 못했던 부분인데 알려주셔서 너무 감사합니다.

코드리뷰 올려주셔서 너무 감사합니다 :)