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

Remove redundant version checks #3849

Conversation

MGaetan89
Copy link
Contributor

Now that the minimum SDK version of Material Components is 19, there was a lot of code that could be simplified or removed.

@@ -47,7 +47,7 @@
@ExperimentalBadgeUtils
public class BadgeUtils {

public static final boolean USE_COMPAT_PARENT = VERSION.SDK_INT < VERSION_CODES.JELLY_BEAN_MR2;
public static final boolean USE_COMPAT_PARENT = false;
Copy link
Contributor Author

@MGaetan89 MGaetan89 Nov 14, 2023

Choose a reason for hiding this comment

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

I didn't know how to proceed here: BadgeUtils is a public class, and so is this USE_COMPAT_PARENT constant.
Can I simply remove USE_COMPAT_PARENT (since BadgeUtils is experimental)? Or should I deprecate it for removal in a future release?

@hunterstich hunterstich self-assigned this Nov 14, 2023
@MGaetan89
Copy link
Contributor Author

Did you have time to have a look at this PR @hunterstich?

@MGaetan89 MGaetan89 force-pushed the remove_redundant_version_checks branch from b8f91bb to ae5e42b Compare January 5, 2024 13:02
@MGaetan89 MGaetan89 force-pushed the remove_redundant_version_checks branch 2 times, most recently from 9528f65 to 7f85347 Compare January 17, 2024 08:00
@MGaetan89 MGaetan89 force-pushed the remove_redundant_version_checks branch from 7f85347 to 40a9bca Compare January 23, 2024 07:33
@MGaetan89 MGaetan89 force-pushed the remove_redundant_version_checks branch from 40a9bca to 8e86a89 Compare January 30, 2024 07:41
@MGaetan89 MGaetan89 force-pushed the remove_redundant_version_checks branch 2 times, most recently from e2af7d5 to 9bb04c1 Compare February 27, 2024 07:46
@MGaetan89
Copy link
Contributor Author

I've fixed the conflict in CarouselLayoutManagerRtlTest

@MGaetan89 MGaetan89 force-pushed the remove_redundant_version_checks branch from 9bb04c1 to 4a034c4 Compare February 27, 2024 07:49
@MGaetan89 MGaetan89 force-pushed the remove_redundant_version_checks branch from a23209a to a573f17 Compare March 26, 2024 15:58
@MGaetan89 MGaetan89 force-pushed the remove_redundant_version_checks branch from a573f17 to ca16c05 Compare April 4, 2024 15:25
@MGaetan89
Copy link
Contributor Author

@hunterstich I've updated the PR and removed some additional legacy code.

@leticiarossi
Copy link
Contributor

Thank you for working on this! I removed the change on the Badge file since it'd be best to refactor badge instead of just having a variable set to false, so that should be done in a separate change.

@dsn5ft dsn5ft closed this in b3fe6a7 May 9, 2024
@MGaetan89 MGaetan89 deleted the remove_redundant_version_checks branch May 10, 2024 06:07
@MGaetan89
Copy link
Contributor Author

Thank you for working on this! I removed the change on the Badge file since it'd be best to refactor badge instead of just having a variable set to false, so that should be done in a separate change.

Are you open for a PR to remove BadgeUtils.USE_COMPAT_PARENT?

@leticiarossi
Copy link
Contributor

sure if you'd like to contribute feel free to do so and we'll happily review it! :)

@MGaetan89
Copy link
Contributor Author

I'll prepare a PR then 🙂
Just to be sure that we're aligned: the goal is to remove BadgeUtils.USE_COMPAT_PARENT and every it's used, correct?

@leticiarossi
Copy link
Contributor

that's right, we should remove the checks for BadgeUtils.USE_COMPAT_PARENT similar to how you removed the checks for API <= 19

@MGaetan89
Copy link
Contributor Author

Ok, #4170 is ready when you have time to have a look 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants