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

fix(android): remove some deprecated classes #14039

Merged
merged 3 commits into from
May 24, 2024
Merged

fix(android): remove some deprecated classes #14039

merged 3 commits into from
May 24, 2024

Conversation

m1ga
Copy link
Contributor

@m1ga m1ga commented May 16, 2024

@m1ga m1ga requested a review from cb1kenobi May 23, 2024 10:38
public static boolean DEBUG = Config.DEBUG;
public static boolean RELEASE = !Config.DEBUG;
public static boolean PROFILE = Config.PROFILE;
public static boolean LOGD = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

The docs say LOGD is always true. Are we sure we want false here? I have no idea what this is even. The Android docs are completely useless.

Just in case, here's the suggestion to expidite the change:

Suggested change
public static boolean LOGD = false;
public static boolean LOGD = true;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Config.LOGD in the Android Config is true, but here we used Config.DEBUG (which is always false) before. And this is the place where it is set:

TiConfig.DEBUG = TiConfig.LOGD = appProperties.getBool("ti.android.debug", false);

with false as default too. So I would keep it as false.
It is added to every module private static final boolean DBG = TiConfig.LOGD; so in case people use that it should be the same value as before (false) unless you change it with the property.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ohhh, that makes sense! Thanks!

Copy link
Contributor

@cb1kenobi cb1kenobi left a comment

Choose a reason for hiding this comment

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

Tested and reviewed. LGTM. APPROVED!

@cb1kenobi cb1kenobi merged commit e1f2dc1 into master May 24, 2024
6 checks passed
@m1ga m1ga deleted the deprecations branch May 29, 2024 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants