Remove all APIs that use framework fragments #206
Conversation
Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>
| @@ -9,6 +9,7 @@ | |||
| /** | |||
| * Permissions helper for {@link Activity}. | |||
| */ | |||
| @Deprecated | |||
| class ActivityPermissionHelper extends BaseFrameworkPermissionsHelper<Activity> { | |||
There was a problem hiding this comment.
This class uses the framework fragments so I deprecated it... not sure if we actually want to do that though.
|
@SUPERCILEX thanks for bringing this up. It's an interesting move for sure, maybe we should wait and see what some other prominent Android libraries do to reflect the change? |
|
@samtstern So I've been poking around a bunch of libraries, and it seems like we're one of the few who support fragments and the only ones who support framework fragments besides https://github.com/permissions-dispatcher/PermissionsDispatcher. I've created permissions-dispatcher/PermissionsDispatcher#445 to see what they think. |
|
@SUPERCILEX wow thanks for doing the research! |
|
@samtstern Ok, so Glide and the other permission library have both deprecated framework fragments so I think that's the way to go. 👍 |
|
@SUPERCILEX thanks for the research! I still want to wait until the actual Fragment deprecation is visible in the SDK (it isn't yet, right) to actually release this. |
|
Alrighty, SGTM |
Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>
# Conflicts: # build.gradle # easypermissions/build.gradle
Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>
# Conflicts: # build.gradle # gradle/wrapper/gradle-wrapper.jar # gradle/wrapper/gradle-wrapper.properties
Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>
SUPERCILEX
changed the title
SUPERCILEX
changed the title
|
@samtstern Ok, API 28 is out! 😁 I nuked all the framework APIs so we can jump straight to v2.0, but I'm still on the fence about the |
| @@ -14,6 +14,7 @@ | |||
| * {@link DialogFragment} to display rationale for permission requests when the request comes from | |||
| * a Fragment or Activity that can host a Fragment. | |||
| */ | |||
| @Deprecated | |||
There was a problem hiding this comment.
What's the reason for marking this internal class @Deprecated? Just for our own knowledge?
There was a problem hiding this comment.
Nope, that was just for me to mark stuff that used the basic Activity. Gonna need to refactor a bit.
|
@samtstern SGTM, that's what I was thinking too. As for support lib stability, it's an RC and they said APIs aren't going to change so I don't think we have too much to worry about there. (Though I don't mind waiting a bit longer. 😄) |
Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>
SUPERCILEX
changed the title
|
Alrighty, this PR is ready to rock whenever you are! 😁 |
|
Nice! I'll give it the respect it deserves on Monday. Also I am not worried about support lib stability, more worried about forcing our consumers to pull an So I think we might want to wait to release until then. That said, can still review and merge in the meantime. |
|
Oh yeah, makes sense. 👍 The support lib team usually ships a final version pretty fast after an RC, so I doubt we'll be waiting too long. 😄 |
|
Yup. We can also get rid of the few remaining deprecated methods. |
@samtstern The Android team announced their decision to kill all framework fragments so this PR deprecates all APIs that use those. It includes eventually getting rid of support for normal activities in some cases. Do you think that's okay?