Skip to content

History / Dagger

Revisions

  • Fix #4119, #4120, and part of #59: Upgrade to Kotlin 1.6.10 (#4937) ## Explanation Fixes #4119 Fixes #4120 Fixes part of #59 This PR finishes the migration of the codebase to Kotlin 1.6 (addressing both #4119 and #4120). Kotlin 1.6 is needed as part of moving rules_kotlin to 1.7.x (which is, in turn, needed in conjunction with Bazel 6.x to enable strict dependency checking which significantly simplifies modularization which is planned for downstream PRs). This PR doesn't actually finish the movement to that version of rules_kotlin, but it does finish moving the codebase to a new enough (and no longer pre-release) version of rules_kotlin to allow using Kotlin 1.6 (over Kotlin 1.4 that the codebase currently uses): version 1.5.0. Previous PRs (#5400 and #5402) prepared for the changes here by addressing large categories of build warnings that have either arisen from this migration, or from past work. Note that another large category of warnings have also been addressed in this PR: by moving to Kotlin 1.6, there's no longer a runtime incompatibility between the Kotlin SDK and the reflection APIs (which was causing a _lot_ of warning output previously). Between all three PRs, the output is now very clean and free of nearly all build warnings. To try and keep the warnings clean long-term, this PR introduces warnings-as-errors for both Java and Kotlin code. However, please note some caveats: - Dagger generated code doesn't follow the Java warnings-as-error flag, so those warnings were cleaned up manually (and will need to be generally watched for, unfortunately). - The version of rules_kotlin used in this PR doesn't directly support turning on the functionality, but does internally (so a small patch file has been added to augment rules_kotlin). When the codebase is updated to rules_kotlin 1.7.x this patch will no longer be needed. - To ease development, a build configuration flag was added to disable failure upon encountering build warnings (per https://bazel.build/run/bazelrc and https://bazel.build/docs/configurable-attributes#query-and-cquery as an example), though this needs to be opted into: ```sh bazel build --config=ignore_build_warnings <target> ``` Some other details to note: - Version 1.6.10 is specifically picked in order to ensure Jetpack Compose compatibility (for preparation of the work being prototyped in #5401 to be compatible with the Oppia Android build environment). - The vast majority of code in this PR is updating parameterized tests to use a cleaner repeatable annotation pattern that wasn't available in Kotlin 1.4. - This upgrade absolutely does have runtime implications, but we're relying very heavily on existing automated tests to ensure correctness and no regressions. - This PR doesn't make an effort to move toward newer Kotlin language features except where forced (API deprecations) or largely wanted (the repeatable annotation change). - android-spotlight and kotlitex have been updated to support newer versions of Kotlin (as both are custom forks managed in the broader Oppia GitHub organization). - Gradle files have been updated to match the same dependency versions as Bazel (where it was obvious to make changes; some might still be a bit off). - The Gradle build configuration was also updated to use Kotlin 1.6.x (otherwise there would be build incompatibilities with Bazel). I think this is the last upgrade we can do for Gradle without upgrading AGP (which will cause us significant issues with the model module, so we're planning on instead dropping Gradle support). - API changes that needed to be addressed in this PR due to deprecations include: ``String.captialize``, ``String.toLowerCase``, ``String.toUpperCase``, ``SendChannel.offer``, and ``Char.toInt``. - New API changes that have been leveraged in this PR: ``Flow.lastOrNull`` and ``Deferred.asListenableFuture`` (to replace ``SettableFuture`` for safety; this also resulted in nice simplifications in ``CoroutineExecutorService``). - The JVM coroutines dependency needed to be split out from Maven and manually imported with some empty internal Java class files since it otherwise has some issues being desugared: https://github.com/bazelbuild/bazel/issues/13553. This is a problem with the Desugarer used in Bazel 4.x (and maybe later versions, so this solution will probably need to kept for a while). - Some Proguard rule updates were needed due to Kotlin SDK changes--see the Proguard file & comments for specifics. - Due to dependency changes, the KitKat main dex file was also trimmed down. I'm fairly certain that it's already crashing on startup, so I don't care much about this change--it just needs to build. We plan to remove KitKat entirely eventually, anyway: #5012. - Jetifier (that is, automatic conversion from support libraries to Jetpack/AndroidX) support was disabled in Gradle. We don't have it enabled in Bazel, and it could potentially encourage strange one-version violations if it was ever actually needed. This is a safer (and likely more performant) change to make. - Moshi was updated to 1.13 to support the upgrade in Kotlin. This did result in a small configuration change due to its annotation processor being moved. Note that Moshi 1.14 couldn't be supported since it requires Kotlin 1.7+ which requires rules_kotlin 1.7+. This will be an option to upgrade in the future. - Some improvements and fixes were made in ``FilterPerLanguageResources`` (I think it was outputting something incorrectly before and that's now been fixed as part of a broader logical reworking of the filtering logic). - ``com.android.support:support-annotation`` was removed as a dependency since it was never used in Bazel, and shouldn't be used (since it's support library and not AndroidX). - The updates to Moshi and Kotlin dependencies resulted in a bunch of other transitive dependency updates. - Note that Gradle doesn't have ``allWarningsAsErrors`` enabled since it would require fixing more warnings than is exposed in Bazel, and we're using Bazel builds as the general source of truth for code quality. ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only N/A -- This is an infrastructural change. While it could inadvertently affect user-facing code, it shouldn't based on the current passing state of automated tests. --------- Co-authored-by: Adhiambo Peres <[email protected]> Co-authored-by: Sean Lip <[email protected]>

    @BenHenning BenHenning committed Jun 12, 2024
  • How to use GitHub’s search feature to search the wiki (#4961) <!-- READ ME FIRST: Please fill in the explanation section below and check off every point from the Essential Checklist! --> ## Explanation How to use GitHub’s search feature to search the wiki - With this contributor easily search and navigate to desire wiki page. - Added TOC in all wikis pages. <!-- - Explain what your PR does. If this PR fixes an existing bug, please include - "Fixes #bugnum:" in the explanation so that GitHub can auto-close the issue - when this PR is merged. --> ## Essential Checklist <!-- Please tick the relevant boxes by putting an "x" in them. --> - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only <!-- Delete these section if this PR does not include UI-related changes. --> If your PR includes UI-related changes, then: - Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes - For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines)) - Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide)) - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

    @MohitGupta121 MohitGupta121 committed May 9, 2023
  • Fix #4622: Android Wiki migration (#4893) <!-- READ ME FIRST: Please fill in the explanation section below and check off every point from the Essential Checklist! --> ## Explanation <!-- - Explain what your PR does. If this PR fixes an existing bug, please include - "Fixes #bugnum:" in the explanation so that GitHub can auto-close the issue - when this PR is merged. --> - Fixes #4622 - The PR does the following: This PR moves the android wiki into the oppia-andoird codebase allowing contributors to make PRs to the wiki along with changes to the codebase itself. This PR also introduces a GitHub Action (Runs when a PR is merged into develop and when a user makes change to the wiki via the web interface) that copies the content of the wiki folder along with the git history of the wiki folder and deploys it to the oppia-android wiki repository. The steps followed to achieve this wiki migration are present here: [Oppia Android Wiki Migration V2 (1).pdf](https://github.com/oppia/oppia-android/files/10921370/Oppia.Android.Wiki.Migration.V2.1.pdf) ### Note: Please ensure GitHub actions have write permissions. Please follow these instructions to enable it: [GrantingReadandWritePermissionsonGitHub_PDF.pdf](https://github.com/oppia/oppia-android/files/10919834/GrantingReadandWritePermissionsonGitHub_PDF.pdf) ## Essential Checklist <!-- Please tick the relevant boxes by putting an "x" in them. --> - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only <!-- Delete these section if this PR does not include UI-related changes. --> If your PR includes UI-related changes, then: - Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes - For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines)) - Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-(A11y)-Guide)) - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing --------- Co-authored-by: Rajat Talesra <[email protected]> Co-authored-by: Ben Henning <[email protected]> Co-authored-by: Akshay Nandwana <[email protected]> Co-authored-by: Srikanth K B <[email protected]> Co-authored-by: Jan <[email protected]> Co-authored-by: Abhay Garg <[email protected]> Co-authored-by: Arjun Gupta <[email protected]> Co-authored-by: Sparsh Agrawal <[email protected]> Co-authored-by: Farees Hussain <[email protected]> Co-authored-by: Sarthak Agarwal <[email protected]> Co-authored-by: Sean Lip <[email protected]> Co-authored-by: Apoorv Srivastava <[email protected]> Co-authored-by: Vinita Murthi <[email protected]> Co-authored-by: Ankita Saxena <[email protected]> Co-authored-by: Vojtěch Jelínek <[email protected]> Co-authored-by: Ben Henning <[email protected]>

    @gp201 gp201 committed Mar 17, 2023