Skip to content

Conversation

@kongwoojin
Copy link
Member

PR 개요

PR 체크리스트

  • Code convention을 잘 지켰나요?
  • Lint check를 수행하였나요?
  • Assignees를 추가했나요?

작업사항

  • 버그 수정
  • 신규 기능
  • 코드 스타일 수정 (포맷팅 등)
  • 리팩토링 (기능 수정 X, API 수정 X)
  • 기타

작업사항의 상세한 설명

  • 이전 PR에서 임시로 비활성화 했던 권한 체크 로직을 다시 추가했습니다.

논의 사항

  • notificationViewModel이 koin 모듈에 있다보니, 결국 notificationViewModel의 코드가 중복되었습니다.
  • 다만, NotificationUiState의 경우, core 모듈로 분리해도 될 것 같다는 생각이 드는데, 어떻게 생각하시는지 궁금합니다.

스크린샷

추가내용

  • develop, sprint 브랜치를 향하고 있습니다
  • production 브랜치를 향하고 있습니다

Copy link
Collaborator

@KYM-P KYM-P left a comment

Choose a reason for hiding this comment

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

notifitcationUiState 에 대해서는 core 모듈로 옮겨도 괜찮을 것 같습니다.
근데 내부 NotificationPermissionInfo 에 대해서는 어떻게 되는걸까요? data 에서도 있고 domain 에서도 참조하던데(내부 repository, usecase)

@kongwoojin
Copy link
Member Author

notifitcationUiState 에 대해서는 core 모듈로 옮겨도 괜찮을 것 같습니다. 근데 내부 NotificationPermissionInfo 에 대해서는 어떻게 되는걸까요? data 에서도 있고 domain 에서도 참조하던데(내부 repository, usecase)

그 점 때문에 core로 옮기지 않았습니다

Base automatically changed from refactor/#1043-move-article-to-article-module to develop January 5, 2026 09:47
@kongwoojin kongwoojin merged commit 2bab75f into develop Jan 5, 2026
6 checks passed
@kongwoojin kongwoojin deleted the feature/#1043-reimplement-permission branch January 5, 2026 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants