Skip to content

Step2#105

Merged
javajigi merged 2 commits into
code-squad:daesoopfrom
daesoop:step2
Dec 29, 2018
Merged

Step2#105
javajigi merged 2 commits into
code-squad:daesoopfrom
daesoop:step2

Conversation

@daesoop
Copy link
Copy Markdown

@daesoop daesoop commented Dec 28, 2018

이번 이슈 생성과 이슈 상세보기를 만들면서 다시 알게 된 것들이 많았습니다.
Issue ATDD를 하는 도중 질문상세보기로 가는 show부분을 테스트 하고 싶었는데, 잘 안되어서, 이 방법 저 방법으로 해보다 안되어 일단 나중으로 미루기로 했습니다.

Html의 중복제거도 굉장히 생각을 많이했습니다. html의 중복 부분들이 분명히 있지만, 중복 되는 부분들이 저번에 qna를 했을 때보다 중복 부분들이 굉장히 적고 불규칙 적이었습니다. 어떻게 하면 중복을 잘 처리할 수 있을지에 대해 고민 후 다음 단계에서 적용해 볼 생각입니다.

Copy link
Copy Markdown
Contributor

@javajigi javajigi left a comment

Choose a reason for hiding this comment

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

테스트 코드 구현까지 잘 했네요. 👍
의견 남겼듯이 다음 단계에서 html 중복 제거해 보세요.
피드백도 남겼으니 다음 단계에 적용해 보세요.

public Issue() {
}

public Issue(@Size(min = 3, max = 50) String subject, @Size(min = 3, max = 500) String content) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

생성자에 굳이 @SiZe는 없어도 괜찮음.

return "issue/show";
}

@GetMapping("list")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

목록의 경우 "/issues"면 되지 않을까? 굳이 list를 추가할 필요가 있을까?

@javajigi javajigi merged commit 94c19e3 into code-squad:daesoop Dec 29, 2018
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