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단계] 헙크(정현승) 미션 제출합니다. #321

Merged
merged 22 commits into from
Sep 5, 2023

Conversation

HubCreator
Copy link
Member

@HubCreator HubCreator commented Sep 4, 2023

푸우 오랜만입니다 ㅎㅎㅎ 조금 급하게 구현하느라 저도 모르게 놓친게 많을 것 같습니다ㅠㅠ 너그러이 봐주세요 ㅎㅎㅎㅎ

Http11Processor를 최대한 가볍게 두고, 객체지향적으로 짜려고 노력했습니다! 미리 고마워요 푸우 :)

Copy link

@BGuga BGuga left a comment

Choose a reason for hiding this comment

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

안녕하세요 ㅎㅎ 간단한 리뷰 남겼습니다!!
코드 리뷰하며 느낀 것이 역할 분배를 굉장히 잘 해주는 것이 느껴졌고 코드도 이해하기 너무 쉬웠네요 ㅎㅎ
수고하셨습니다!!

tomcat/src/main/java/org/apache/coyote/http11/Handler.java Outdated Show resolved Hide resolved
Comment on lines 18 to 22
return new Response(
StatusLine.of(httpStatus),
ResponseHeader.of(responseBody),
responseBody
);
Copy link

Choose a reason for hiding this comment

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

여기서 Response 를 만들기 위해 HttpStatusCode 와 ResponseBody 를 받는데 HttpResponse 생성에 대한 전적인 책임을 지기 위해서는 HttpHeaders 도 같이 받는 것이 어떤가 의견내봅니다 ㅎㅎ ☺️

Copy link

@BGuga BGuga left a comment

Choose a reason for hiding this comment

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

안녕하세요 ㅎㅎ 간단한 리뷰 남겼습니다!!
코드 리뷰하며 느낀 것이 역할 분배를 굉장히 잘 해주는 것이 느껴졌고 코드도 이해하기 너무 쉬웠네요 ㅎㅎ
수고하셨습니다!!

@HubCreator
Copy link
Member Author

리뷰주셔서 감사합니다 푸우! 빠르게 넘어가려고만 했던 부분들에 대해서 푸우 덕분에 되돌아볼 수 있었던 것 같아요 ㅎㅎㅎ

여러 방어 로직이나, 엣지 케이스에 대해서 생각해볼 수 있었습니다. 빠르게 리뷰주셔서 정말 감사하고 또 얘기나눠봤으면 좋겠네요 ㅎㅎㅎ

Copy link

@BGuga BGuga left a comment

Choose a reason for hiding this comment

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

안녕하세요 헙크 피드백 반영하신 것 잘 봤습니다!!
추가적으로 몇 가지 사항에 대해서 이야기 해보고 다음 단계로 넘어가면 좋을 것 같습니다 ㅎㅎ
수고하셨습니다👍

@sonarcloud
Copy link

sonarcloud bot commented Sep 5, 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

@BGuga BGuga merged commit aea21d0 into woowacourse:hubcreator Sep 5, 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