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 and Check warnings / deprecation notes in the current build #811

Merged
merged 6 commits into from Jul 5, 2023

Conversation

HollowMan6
Copy link
Collaborator

  • Enable showing all build warnings
  • Fix build and deprecation warnings and add some TODO comments for deprecation (Because they may not be feasible at the moment or need refactoring. e.g. AsyncTask, JobIntentService).

There are also some warnings that may be fixed by upgrading the dependency versions, and they are not fixed in this PR:

> Task :app:gleanGenerateMetricsSourceForAospArm64GeckoGenericDebug
Glean SDK - generating API from /wolvic/app/metrics.yaml
Glean SDK - generating API from /wolvic/app/pings.yaml
The AbstractExecTask.execResult property has been deprecated. This is scheduled to be removed in Gradle 8.0. Please use the executionResult property instead. See https://docs.gradle.org/7.3.2/dsl/org.gradle.api.tasks.AbstractExecTask.html#org.gradle.api.tasks.AbstractExecTask:execResult for more details.
> Task :app:compressAospArm64GeckoGenericDebugAssets
warn: removing resource com.igalia.wolvic:string/mozac_browser_errorpages_page_go_back without required default value.
warn: removing resource com.igalia.wolvic:string/mozac_browser_errorpages_page_title without required default value.
warn: removing resource com.igalia.wolvic:string/mozac_feature_addons_user_rating_count without required default value.
> Task :app:stripAospArm64GeckoGenericDebugDebugSymbols
Unable to strip the following libraries, packaging them as they are: libjnidispatch.so.
> Task :app:compileAospArm64GeckoGenericDebugJavaWithJavac
ANTLR Tool version 4.5.3 used for code generation does not match the current runtime version 4.7.1

WARNING: Since I still haven't got my testing device, the code in bundled JDK 17 using Android Studio Flamingo.

Copy link
Member

@svillar svillar left a comment

Choose a reason for hiding this comment

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

Just some nits.

Copy link
Member

@svillar svillar left a comment

Choose a reason for hiding this comment

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

Awesome.

Do please integrate the review fixes in the other commits and then we'll be ready to merge.

@HollowMan6
Copy link
Collaborator Author

I just squashed those review fixes commmits

Do please integrate the review fixes in the other commits and then we'll be ready to merge.

Copy link
Member

@svillar svillar left a comment

Choose a reason for hiding this comment

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

Also as a general comment please do include the link to the issues you've created for the other dependency issues in the comments in the code. Instead of repeating "this is deprecated since..." we could better simply add "See URL_TO_THE_ISSUE". The comments will be shorter as the detailed explanation is in the issue, and we'll have traceability.

Signed-off-by: Songlin Jiang <sjiang@igalia.com>
PackagingOptions.jniLibs.useLegacyPackaging should be set to
 true because android:extractNativeLibs is set to "true"
 in AndroidManifest.xml.

Signed-off-by: Songlin Jiang <sjiang@igalia.com>
- w: wolvic/app/src/common/gecko/com/igalia/wolvic/browser/api/impl/GeckoViewFetchClient.kt:26:5 Parameter 'context' is never used
- w: wolvic/app/src/common/gecko/com/igalia/wolvic/browser/api/impl/GeckoWebExtension.kt:272:29 Unnecessary safe call on a non-null receiver of type WebExtension.MetaData
- w: wolvic/app/src/common/shared/com/igalia/wolvic/browser/adapter/ComponentsAdapter.kt:88:15 This declaration needs opt-in. Its usage should be marked with '@kotlinx.coroutines.ExperimentalCoroutinesApi' or '@OptIn(kotlinx.coroutines.ExperimentalCoroutinesApi::class)'

Signed-off-by: Songlin Jiang <sjiang@igalia.com>
Signed-off-by: Songlin Jiang <sjiang@igalia.com>
Signed-off-by: Songlin Jiang <sjiang@igalia.com>
@HollowMan6
Copy link
Collaborator Author

Done! Ready to get merged again.

Fix #810
Gecko has deprecated cookieLifetime as
"this feature is not available anymore"
since v103, and we are now using v103
https://github.com/Igalia/gecko-dev/tree/FIREFOX_103_0_2_RELEASE
cookieLifetime can be safely removed.

Signed-off-by: Songlin Jiang <sjiang@igalia.com>
Copy link
Member

@svillar svillar left a comment

Choose a reason for hiding this comment

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

Really happy to merge this!

@svillar svillar merged commit 49900f5 into main Jul 5, 2023
7 checks passed
@svillar svillar deleted the deprecated branch July 5, 2023 17:17
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.

None yet

2 participants