-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat/31-withdraw-member #32
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.
회원탈퇴 기능 전부 확인했습니다. 수고하셨습니다!
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.
고생하셨습니다!!
…t/31-withdraw-member # Conflicts: # src/main/java/com/sptp/backend/member/service/MemberService.java
…t/31-withdraw-member # Conflicts: # src/main/java/com/sptp/backend/member/service/MemberService.java # src/main/java/com/sptp/backend/member/web/MemberController.java
|
||
return Member.builder() | ||
.userId("test") | ||
.password("12345678") |
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.
Member의 여러 필드 중 userId랑 password필드를 넣어주신 건 추후 테스트를 위함인가요?
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.
전부 null값을 가져도 되지만, userId와 password 정도만 임의로 추가했습니다. 제거해도 무방합니당
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.
넵!
src/test/resources/application.yml
Outdated
datasource: | ||
url: ${application.spring.datasource.url} | ||
username: ${application.spring.datasource.username} | ||
password: ${application.spring.datasource.password} |
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.
이거 test/resources에 env.properties파일 추가하셔서 적용하신건가요? 그렇다면 혹시 기존에 main/resources에 있는 env.properties의 변수명과 같은데 문제 없나요?? 전 env.properties파일의 변수가 환경변수로 등록되는 걸로 알고 있어서요!
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.
아 test/resources에 파일 새로 생성한거 맞습니다! main에 있는 폴더와 다른 점은 url이 h2를 가르킨다는 점이에용
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.
main의 env파일에서 환경변수로 등록한 것과 변수명이 같은데 괜찮나용? application.spring.datasource.url 변수명을 다르게 해야하지 않나 싶어서요! 둘 중 하나는 값이 덮어 씌어지지 않을까 싶어요
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.
변수명은 사실 어떤 걸 써도 무방해서 기존 main의 env파일에서 등록했던 환경변수명과 다르게 네이밍 해주시면 좋을거같아요-! test.application.spring.datasource.url처럼 앞에 test만 붙여주셔도 괜찮구용
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.
바꾼다면 url 뿐만 아니라 Oauth와 같은 것들도 모두 test를 붙인 변수명으로 수정하는 것이 좋다고 생각합니다.
하지만 이미지와 같이 @value에서 test가 없는 변수명을 요구하는 클래스가 있기 때문에 main/resources/application.yml과 변수명을 달리하면 에러가 발생하네요.
또한, test/resouces 하위에 application.yml이 존재하는 경우에는, main/resources/application.yml이 무시됩니다!
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.
그것도 괜찮을 거 같아용 비밀번호 저렇게 입력 안 해줘도 알아서 h2 구동이 되는건가요??
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.
그럼 h2관련 부분은 빼는 걸로 합시당
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.
수정하고 바로 머지할께욥
📌 관련 이슈
#31
✨ 작업 내용