DND Code Review changes #449
Conversation
|
Could you rebase this on top of master? Looks like this PR contains both your old & new work, while it should only contain your new work. :-) Thanks. |
|
Ah, I just realized it's a single commit, I just did it for you. |
I like to believe I'm quite good at testing (I have python/rust/go projects where test coverage is almost complete) and Android testing is quite hard. :-) So no problem, I just tried my luck. |
| val filterAll = NotificationManager.INTERRUPTION_FILTER_ALL | ||
| dndManager.setInterruptionFilter(preferences.getInt("current_dnd", filterAll)) | ||
| if (dndManager.isNotificationPolicyAccessGranted) { | ||
| if (dndEnabled) { |
There was a problem hiding this comment.
I think you got the order wrong here: my suggestion was:
if (enabled) {
if (havePermission) {
...
} else {
... warn ...
}
}
| @@ -30,76 +20,6 @@ class Preferences : PreferenceFragmentCompat() { | |||
| it.summary = path | |||
| } | |||
| } | |||
|
|
|||
| override fun onPreferenceTreeClick(preference: Preference): Boolean { | |||
There was a problem hiding this comment.
But this one looks quite nice, good to get rid of this.
Don't warn in default config
Yeah sorry about that, I thought it would of done that automatically.
If I'm correct you mean to remove the |
|
The warnings are gone in the default config, thanks. |
I implemented all (if not most of) the suggestions you made in the code review at pull request #448
By the way sorry about the fact there are no tests, I'm mostly a beginner with Android development and don't have much experience with testing in general as well. But hopefully these changes find you well!