-
Notifications
You must be signed in to change notification settings - Fork 305
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
[MVC 구현하기 - 2,3단계] 케빈(박진홍) 미션 제출합니다. #102
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.
고생하셨어요 케빈! 코드 상으로는 크게 이상한 부분이 보이지 않았어요. 다만, 새롭게 만든 컨트롤러의 이름이 New___ 라는건 조금 오해의 소지가 있을 수도 있을 것 같아요. 그런데 저도 마땅한 대안이 떠오르지는 않네요. 그 외에 코드상 바뀐 부분에 대한 설명이랑 jackson
버전이 올라간 부분에 대한 설명이 없는 것 같아 코멘트 달았습니다.
@@ -0,0 +1,17 @@ | |||
package com.techcourse.controller.annotation_base; |
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.
패키지 명에 언더바가 들어가 있어요. 물론 컨벤션이나 기타 제약, 암묵적인 합의 등을 고려하셔서 짠 부분이겠지만 구글 컨벤션 스타일에서는 해당 스타일을 지양하도록 되어 있어서 관련 문서 하나 남기겠습니다.
https://stackoverflow.com/questions/3179216/what-is-the-convention-for-word-separator-in-java-package-names
SonarCloud Quality Gate failed. |
안녕하세요 마갸, 피드백 반영했습니다. |
확인했습니다! 고생하셨어요. 버전 관련해서는 코어를 옮긴 부분이니 이유에 대해서도 어느정도 명시되어 있으면 좋을것 같아서 드린 말씀이었는데 저도 왜 버전을 하나 올려야하는지는 잘 모르겠더라구요... 공식 문서에서도 핫픽스 정도만 바뀐 정도였거든요. 어찌됬든 고생 많으셨습니다! 여러 악재가 겹쳐서 답변 늦어진 부분이 죄송하네요. 다음 미션도 많이 배워갔으면 좋겠습니다. |
안녕하세요 마갸 잘부탁드립니다.
크게 변경사항은 없구요
json view에 대해서 테스트를 진행하려고 했는데 request, response 때문에 어플리케이션 실행후 수동 확인밖에 못했습니다 ㅠㅠ