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.

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

Feature: Timer Service Refactoring #140

Merged
merged 6 commits into from
Oct 23, 2024
Merged

Conversation

lee-ji-hoon
Copy link
Collaborator

@lee-ji-hoon lee-ji-hoon commented Oct 19, 2024

작업 내용

체크리스트

  • 빌드 확인

동작 화면

살려주세요


LocalNotificationReceiver나 다른 Service는 다음 PR에 나눠서 올릴 예정

작업 내용

  • Timer 비즈니스 로직을 각각의 클래스로 분리
  • 하나의 Service가 아닌 Focus, Rest Service로 분리
    • 이전처럼 STOP / START가 서로 꼬일 일이 없음=

체크리스트

  • 빌드 확인
  • 집중 / 휴식의 기능이 정상적으로 사용이 되는지

동작 화면

살려주세요

@lee-ji-hoon lee-ji-hoon added the refactor 기능은 그대로면서, 버전 혹은 코드 일부 변경 label Oct 19, 2024
@github-actions github-actions bot requested a review from HyomK October 19, 2024 06:36
import timber.log.Timber

internal class PomodoroNotificationManager @Inject constructor(
@PomodoroNotification private val notificationBuilder: NotificationCompat.Builder,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

어제 밤에 이야기 했던 NotificationBuilder 를 hilt를 아직 유지한 이유는 app과 관련된 정보를 presenter에서 알 수가 없다 보니 우선 유지하고 해당 정보들을 어떻게 할지 좀 고민해볼게

Copy link
Member

Choose a reason for hiding this comment

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

알림 추가 기능 확인하고 변경해도 괜찮을듯!

Comment on lines -42 to -43
private var focusTimer: Timer? = null
private var restTimer: Timer? = null
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

하나의 Service에서 2개의 Timer 로직을 갖고 있으니 구분도 어렵고, 코딩을 할 때 실수가 잦아져서 구성 자체를 변경

Copy link
Member

@HyomK HyomK left a comment

Choose a reason for hiding this comment

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

타이머 분리하니까 확실히 가독성이 좋아졌군요 bb

android:name=".presentation.service.focus.PomodoroFocusTimerService"
android:foregroundServiceType="specialUse">
<meta-data
android:name="android.app.PROPERTY_SPECIAL_USE_FGS_SUBTYPE"
Copy link
Member

Choose a reason for hiding this comment

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

Android 14 대응이구나
가이드 보니까 meta-data 말고 property 태그를 사용하던데 차이가 있을까?
https://developer.android.com/about/versions/14/changes/fgs-types-required

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

meta-data 가 익숙해서 사용한거였는데, 찾아 보니 property 는 Android 14부터 도입된 개념으로 나오네 이 방법으로 해동 상관 없을거 같아 변경해두고 다시 멘션할게!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

import timber.log.Timber

internal class PomodoroNotificationManager @Inject constructor(
@PomodoroNotification private val notificationBuilder: NotificationCompat.Builder,
Copy link
Member

Choose a reason for hiding this comment

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

알림 추가 기능 확인하고 변경해도 괜찮을듯!

@lee-ji-hoon lee-ji-hoon merged commit 22ce317 into develop Oct 23, 2024
1 check passed
@lee-ji-hoon lee-ji-hoon deleted the feature/service-refactor branch October 23, 2024 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor 기능은 그대로면서, 버전 혹은 코드 일부 변경
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants