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

Feature/dnd special #169

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Feature/dnd special #169

wants to merge 4 commits into from

Conversation

alertjjm
Copy link
Member

  • 일단 개발자용이니까 테스트작성안하고 개발부터 했습니다!

@alertjjm alertjjm requested a review from Catnap421 August 19, 2021 01:48
@alertjjm alertjjm temporarily deployed to production August 19, 2021 01:48 Inactive
@github-actions
Copy link

github-actions bot commented Aug 19, 2021 •
edited
Loading

[unit-test] Results

19 files  ±0  19 suites  ±0   1s ⏱️ ±0s
43 tests ±0  43 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit e2d01c8. ± Comparison against base commit 3976875.

♻️ This comment has been updated with latest results.

@alertjjm alertjjm temporarily deployed to production August 19, 2021 02:30 Inactive
@Catnap421
Copy link
Collaborator

@alertjjm 이거 다 된거야??

@alertjjm
Copy link
Member Author

@Catnap421 AggregationService에서 로직상 initWordCount하는게 assignCard속에 포함되어야해서 그거 포함시켰는데 테스트 코드에서 모킹을 안해놔서 에러떴었나봐 ㅋㅋ

Catnap421
Catnap421 requested changes Aug 19, 2021
View reviewed changes
Copy link
Collaborator

@Catnap421 Catnap421 left a comment

Choose a reason for hiding this comment

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

special 카드는 대상이 정해지는 경우가 많다고 생각돼서 나는 cardId를 받아서 할당할까했었거든??
지금 당장엔 랜덤 방식이 괜찮지만, 스페셜의 목적에 맞으려면 카드를 지정해서 넘겨줄 수 있어야 할 거 같아

@alertjjm
Copy link
Member Author

special 카드는 대상이 정해지는 경우가 많다고 생각돼서 나는 cardId를 받아서 할당할까했었거든??
지금 당장엔 랜덤 방식이 괜찮지만, 스페셜의 목적에 맞으려면 카드를 지정해서 넘겨줄 수 있어야 할 거 같아

아 그러네 ㅋㅋㅋ 오케오케 수정해야겠다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Catnap421 Catnap421 Catnap421 requested changes

Requested changes must be addressed to merge this pull request.

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
@alertjjm @Catnap421