-
Notifications
You must be signed in to change notification settings - Fork 309
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
[톰캣 구현하기 - 3, 4단계] 헙크(정현승) 미션 제출합니다. #421
Conversation
There was a problem hiding this 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/controller/AbstractController.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/response/StaticResource.java
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/controller/Controller.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/controller/IndexController.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/controller/IndexController.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/response/ResponseBody.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/response/ResponseBody.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/response/ResponseHeaders.java
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/response/HttpResponse.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 헙크!!🫡
코드 깔끔하게 잘 짜주셔서 리뷰하기 너무 수월했습니다 ㅎㅎ
읽으면서 궁금한 것들을 질문으로 남겨뒀는데 답변 주시면 감사하겠습니다!!
좋은 주말 보내세요 ㅎㅎ
푸우우 좋은 피드백 주셔서 감사합니다ㅎㅎㅎ 푸우의 피드백과 더불어, 슬랙에서 확인해보니 주어진 Controller와 AbstractController를 수정하면 안된다는 것을 뒤늦게 확인했습니다. 그래서 요구사항에 맞게 다시 전체적인 구조를 수정하였습니다. 감안하고 봐주시면 좋을 것 같네요 ㅎㅎㅎ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 헙크!!
리뷰 반영, 답변 확인했습니다 ㅎㅎ
이미 잘 구현해주셔셔 approve 할까 고민했지만 아직 헙크와 조금 더 의견을 나눠보고 싶네요 !!
헙크 덕분에 많이 고민하고 배워가네요 감사합니다 ㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추가 커멘트 날립니다!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 헙크~!
리뷰 답변 주신 것, 반영하신 것 확인했습니다 ㅎㅎ
이번 미션 너무너무 잘해주셔서 이만 approve 해도 될 것 같네요 😀
수고 많으셨습니다 ㅎㅎ
Kudos, SonarCloud Quality Gate passed! 0 Bugs 0.0% Coverage 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. |
안녕하세요 푸우우. 다시 돌아왔습니다 ㅎㅎㅎ
기존 구조에서 크게 달라진 것은 없고, 3단계에서 Controller 구조를 도입한 부분만 크게 달라졌습니다. 추가적으로 테스트를 넣어봤습니다. 감사합니다 :)