Skip to content
This repository was archived by the owner on Feb 9, 2023. It is now read-only.

Conversation

SeoJungHong
Copy link

@SeoJungHong SeoJungHong commented Nov 16, 2018

No description provided.

@SeoJungHong SeoJungHong requested a review from ssowonny November 16, 2018 05:51
Copy link

@ssowonny ssowonny left a comment

Choose a reason for hiding this comment

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

본 repo 를 보니 아무래도 운영이 잘 안되고 있는 것 같기도 하고, 그래서인지 outdated 된 부분이 많아서 수정사항이 많은 것 같긴 하네요. 이 부분 혹시 최소한으로 수정해서 google repo 에 PR 날려보실 생각은 없으실까요?ㅎㅎ

import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.support.v7.app.NotificationCompat;
import android.support.v4.app.NotificationCompat;

Choose a reason for hiding this comment

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

min sdk target 이 몇시요? v7 으로 사용해도 괜찮지 않을까요?

Copy link
Author

Choose a reason for hiding this comment

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

android.support.v7.app.NotificationCompat 은 android.support.v4.app.NotificationCompat 을 상속받아서 쓰고 deprecate 되어 있었는데 support lib version 이 26 에서 27 로 올라가면서 아예 삭제되어서 빌드 에러가 났습니다. https://stackoverflow.com/questions/38249970/how-to-import-android-support-v7-app-notificationcompat-builder-class-in-android 여기도 참고했고요. 가이드에서는 언제 삭제되었는지 히스토리를 찾을 수가 없었지만 일단 현재는 https://developer.android.com/reference/android/support/v4/app/NotificationCompat.Builder 이렇게 v4 의 NotificationCompat 만 찾을 수가 있었습니다.

}

private void changeState(@NonNull HoverViewState nextState) {
Log.d(TAG, "Giving up control.");

Choose a reason for hiding this comment

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

삭제 부탁 드립니다

@SeoJungHong
Copy link
Author

@ssowonny 네! 저도 저희 내부에서 리뷰가 끝나면 그쪽에도 PR을 보낼 생각으로 브랜치를 나누어서 가장 최소한의 업데이트 및 버그픽스만 따로 PR을 날리게 되었어요. gradle, support lib도 너무 오래된 버전인것 같아서 관리 차원에서 올린 것인데 이런것 빼고 버그픽스만 따로 보내는 것이 좋을까요?

@SeoJungHong SeoJungHong changed the title Update build settings, Fix NPE Update build settings Nov 23, 2018
@SeoJungHong SeoJungHong changed the title Update build settings Update gradle version, build properties Nov 23, 2018
@SeoJungHong SeoJungHong changed the base branch from master to fix/npe_changing_state November 23, 2018 09:00
Copy link

@ssowonny ssowonny left a comment

Choose a reason for hiding this comment

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

👍

@SeoJungHong SeoJungHong merged commit 4b72d78 into fix/npe_changing_state Dec 3, 2018
@SeoJungHong SeoJungHong deleted the feature/fix_errors branch December 3, 2018 09:15
@SeoJungHong SeoJungHong restored the feature/fix_errors branch December 3, 2018 09:17
SeoJungHong added a commit that referenced this pull request Dec 3, 2018
@SeoJungHong SeoJungHong deleted the feature/fix_errors branch December 3, 2018 09:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants