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.

Sign up for GitHub

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

Jump to bottom

[톰캣 구현하기 1, 2단계] 오도(문민혁) 미션 제출합니다. #344

Merged
merged 20 commits into from
Sep 7, 2023
Merged

[톰캣 구현하기 1, 2단계] 오도(문민혁) 미션 제출합니다. #344

merged 20 commits into from
Sep 7, 2023

Conversation

odo27
Copy link

@odo27 odo27 commented Sep 4, 2023

안녕하세요 루쿠!
리뷰 잘부탁드립니다~

@sonarcloud
Copy link

sonarcloud bot commented Sep 4, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 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

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

aiaiaiai1
aiaiaiai1 requested changes Sep 5, 2023
View reviewed changes
Copy link

@aiaiaiai1 aiaiaiai1 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

안녕하세요 오도~
미션 깔끔하게 구현해주셨네요!
전체적으로 기능에 대한 책임과 역할을 클래스로 잘 분리했다고 느껴지는 코드였습니다!
if문에 대한 분기 처리는 3단계 리펙터링때 개선하시면 될거 같습니다
몇가지 코멘트와 질문 남겨드렸는데 확인한번 부탁드려요~!
미션 고생하셨습니다 👍

tomcat/src/main/java/org/apache/coyote/http11/ExceptionHandler.java Outdated Show resolved Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/HttpResponse.java Outdated Show resolved Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/ResourceLoader.java Outdated Show resolved Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/HttpRequest.java Show resolved Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/Http11Processor.java Outdated Show resolved Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/Http11Processor.java Outdated Show resolved Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/HttpCookie.java Outdated Show resolved Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/FormData.java Show resolved Hide resolved
@odo27
Copy link
Author

odo27 commented Sep 7, 2023

안녕하세요 루쿠! 꼼꼼한 리뷰 감사합니다 👍
리뷰해주신 부분 반영했고 HttpCookie 사용한 이유와 HttpException에서 Status 참조하는 부분에 관한 질문 남겼습니다!

@sonarcloud
Copy link

sonarcloud bot commented Sep 7, 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 1 Code Smell

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

@aiaiaiai1
Copy link

안녕하세요 오도~
리뷰 반영해주신거 확인했습니다
3,4 단계 미션 진행하시면 될거같아요!
다음 미션도 화이팅입니다 😄

@aiaiaiai1 aiaiaiai1 merged commit adc6725 into woowacourse:odo27 Sep 7, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aiaiaiai1 aiaiaiai1 aiaiaiai1 requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@odo27 @aiaiaiai1