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

[MVC 구현하기 - 2단계] 프린(남기범) 미션 제출합니다. #730

Merged
merged 10 commits into from
Sep 30, 2024
Merged

[MVC 구현하기 - 2단계] 프린(남기범) 미션 제출합니다. #730

merged 10 commits into from
Sep 30, 2024

Conversation

GIVEN53
Copy link
Member

@GIVEN53 GIVEN53 commented Sep 24, 2024

안녕하세요 짱짱수 :) 2단계도 잘 부탁드립니다

notice

#690 (comment) 1단계 리뷰를 반영하기 위해 애플리케이션 로드 타임때 모든 bean 등록을 구현하려고 했으나 배보다 배꼽이 더 커지는 상황이 오더라구요
요구사항에만 집중하자는 생각으로 BeanFactory는 삭제했습니다

@GIVEN53 GIVEN53 requested a review from zangsu September 24, 2024 12:03
@GIVEN53 GIVEN53 self-assigned this Sep 24, 2024
zangsu
zangsu requested changes Sep 25, 2024
View reviewed changes
Copy link

@zangsu zangsu left a comment

Choose a reason for hiding this comment

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

프린은 예외 처리가 정말 꼼꼼해서 코드를 볼 때 마다 '본받아야겠다.' 라는 생각이 들어요.
코드도 깔끔하게 잘 읽혀서 쉽게 리뷰할 수 있었습니다 ^^

코멘트 반영하면서 DispatcherServlet 의 구조가 어느 정도 바뀔 것 같아요.
그 때 조금 더 이야기를 나누어 보는 걸로 하죠!

app/src/main/java/com/techcourse/controller/RegisterController.java Show resolved Hide resolved
app/src/main/java/com/techcourse/ManualHandlerMapping.java Show resolved Hide resolved
mvc/src/main/java/com/interface21/webmvc/servlet/mvc/tobe/AnnotationHandlerMapping.java Outdated Show resolved Hide resolved
mvc/src/main/java/com/interface21/webmvc/servlet/mvc/HandlerAdapter.java Show resolved Hide resolved
mvc/src/test/java/com/interface21/webmvc/servlet/mvc/tobe/AnnotationHandlerMappingTest.java Outdated Show resolved Hide resolved
app/src/main/java/com/techcourse/DispatcherServlet.java Show resolved Hide resolved
app/src/main/java/com/techcourse/DispatcherServlet.java Show resolved Hide resolved
app/src/main/java/com/techcourse/DispatcherServlet.java Outdated Show resolved Hide resolved
GIVEN53
GIVEN53 commented Sep 29, 2024
View reviewed changes
Copy link
Member Author

@GIVEN53 GIVEN53 left a comment

Choose a reason for hiding this comment

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

안녕하세요 짱수!
데모데이 준비하느라 바빠서 코멘트가 많이 늦었네요 😂

리뷰 반영한 부분 및 코멘트 확인부탁드려요!

app/src/main/java/com/techcourse/controller/RegisterController.java Show resolved Hide resolved
app/src/main/java/com/techcourse/DispatcherServlet.java Show resolved Hide resolved
app/src/main/java/com/techcourse/ManualHandlerMapping.java Show resolved Hide resolved
mvc/src/main/java/com/interface21/webmvc/servlet/mvc/tobe/AnnotationHandlerMapping.java Outdated Show resolved Hide resolved
app/src/main/java/com/techcourse/DispatcherServlet.java Outdated Show resolved Hide resolved
zangsu
zangsu requested changes Sep 29, 2024
View reviewed changes
Copy link

@zangsu zangsu left a comment

Choose a reason for hiding this comment

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

데모데이 끝나고 열심히 반영해 주셨네요!
고생하셨습니다~~~

회원가입 페이지 조회 부분만 다시 확인해 주세요!

app/src/main/java/com/techcourse/DispatcherServlet.java Show resolved Hide resolved
app/src/main/java/com/techcourse/ManualHandlerMapping.java Show resolved Hide resolved
app/src/main/java/com/techcourse/controller/RegisterController.java Show resolved Hide resolved
mvc/src/main/java/com/interface21/webmvc/servlet/mvc/tobe/AnnotationHandlerMapping.java Outdated Show resolved Hide resolved
zangsu
zangsu approved these changes Sep 30, 2024
View reviewed changes
Copy link

@zangsu zangsu left a comment

Choose a reason for hiding this comment

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

2단계도 고생 많으셨습니다~
3단계도 빠르게 달려보시죠 🔥

@zangsu zangsu merged commit 79eab62 into woowacourse:given53 Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zangsu zangsu zangsu approved these changes

Assignees

@GIVEN53 GIVEN53

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@GIVEN53 @zangsu