Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[톰캣 구현하기 - 1,2단계] 제나(위예나) 미션 제출합니다. #355

Merged
merged 35 commits into from
Sep 9, 2023

Conversation

yenawee
Copy link

@yenawee yenawee commented Sep 4, 2023

안녕하세요 로건!

레벨 4 첫 미션 리뷰어로 만나게 되어서 정말 반가워요 🙌

기능을 구현하고 리팩터링할 생각에 Request, Response 등 객체 분리도 못하고 다 HttpProcess 에 코드가 전부 담겨있어서 이게 뭔가 싶을거같아 걱정됩니다.... 제가 봐도 이게 뭔가 싶네여....

리뷰 바탕으로 리팩터링 진행하고 있겠습니다

리뷰 잘부탁드립니다~~!!

@yenawee yenawee self-assigned this Sep 4, 2023
Copy link
Member

@70825 70825 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 제나~

아래에 빠진 기능이 있어서 추가해주시고, 3~4단계 진행하면서 같이 리팩터링을 진행하면 될 것 같아요!

  • 회원가입 로직이 아예 없는 것 같아요
  • 프로그램 실행하고, 웹페이지 들어가면 널포인트에러가 나오는데 Content-Type 수정하면 해결될 것 같습니다~

기능이 작동하지 않는 부분만 우선적으로 고쳐주시고, 나머지 리뷰는 리팩터링할 때 같이 진행하고 싶은 부분은 댓글 남겨주시면 됩니다~

@sonarcloud
Copy link

sonarcloud bot commented Sep 9, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 8 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

warning The version of Java (11.0.20.1) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

@yenawee
Copy link
Author

yenawee commented Sep 9, 2023

로건 안녕하세요

리팩터링이 안된채로 작동되지 않는 코드들 수정하려다보니 못하겠어서
리팩터링 먼저 진행하느라 리뷰 요청이 많이 늦었습니다,,,,

최대한 객체분리를 해봤어요!
url 을 따라 컨트롤러 매핑되는 것은 아직 구현 전입니다!! 👩🏻‍💻

테스트 코드도 없지만 리뷰 요청이 너무 늦어질거같아서 리뷰 요청 드립니다
리뷰 감사합니다 ~~

Copy link
Member

@70825 70825 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 제나~ 1, 2단계 하느라 고생하셨어요!
404, 500 관련 처리들은 3, 4단계 진행하면서 같이 추가하면 될 것 같아요
나머지는 3단계 진행하다가 리팩터링할 때 깜빡할 수도 있는 내용들 위주로 리뷰 적어두었습니다~
내일 빠르게 미션 진행할 수 있도록 머지할게요!

@70825 70825 merged commit 89596ca into woowacourse:yenawee Sep 9, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants