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

feat(socketService): 소켓 서비스 추가 #206

Merged
merged 6 commits into from
Dec 11, 2023
Merged

feat(socketService): 소켓 서비스 추가 #206

merged 6 commits into from
Dec 11, 2023

Conversation

vimkim
Copy link
Collaborator

@vimkim vimkim commented Dec 11, 2023

Checklist

  • Code Review: 작성한 코드를 다시 한 번 꼼꼼이 확인했나요?
  • Testing: 앱이 잘 구동되는지 개발한 기능이 문제 없이 작동하는지 확인했나요?
  • Remove: print나 주석 등 필요없는 코드를 삭제했나요?
  • Rebase: (필요시) rebase를 완료했나요?
  • Conflict Resolution: 충돌을 해결하는 과정을 거쳤나요?
  • New Dependencies: 새로운 dependency를 추가했나요?

Description

  • 길어진 SocketGateway 코드를 SocketService로 분리
    • 공식 문서를 참고하여 afterInit으로 SocketService에 server 주입했습니다.
  • 소켓과 http 분리
    • socket connect와 http create / join
    • socket disconnect와 http exit 차이점 명확하게 분리

Changes Made

Extra Comments

Demo

sjn0910
sjn0910 previously approved these changes Dec 11, 2023
View reviewed changes
Copy link
Collaborator

@sjn0910 sjn0910 left a comment

Choose a reason for hiding this comment

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

GOOD

server/src/entities/room.entity.ts Show resolved Hide resolved
server/src/socket/socket.gateway.ts Outdated Show resolved Hide resolved
sjn0910
sjn0910 approved these changes Dec 11, 2023
View reviewed changes
Copy link
Collaborator

@sjn0910 sjn0910 left a comment

Choose a reason for hiding this comment

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

LGTM!

@vimkim vimkim merged commit 034c40f into develop Dec 11, 2023
1 check passed
@vimkim vimkim deleted the feat/socket-service branch December 11, 2023 07:01
@vimkim vimkim self-assigned this Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjn0910 sjn0910 sjn0910 approved these changes

@Lukaid-dev Lukaid-dev Awaiting requested review from Lukaid-dev

Assignees

@vimkim vimkim

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vimkim @sjn0910