-
Notifications
You must be signed in to change notification settings - Fork 520
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#3146 : Create a generic utility for filtering enums #5529
base: develop
Are you sure you want to change the base?
Conversation
…ring enums . I have changed the following files UserTypeitemsViewModel , PerformancemetricsController and TopicLessonsFragmentPresenter.The function i made is filterByEnumCondition - This function is used to filter thelist of items based on the enum condition.
Please help me to make this PR work and review my changes. |
Hi @whyash8, please make sure you have signed the CLA here - Individual CLA. |
I have signed the CLA !please help me to proceed further . thanks |
Hi @whyash8, bazel tests are failing most likely because the new utility should be added as a dependency in app/BUILD.bazel. To see the test failures, please read https://github.com/oppia/oppia-android/wiki/Static-Analysis-Checks#developer-video---understanding-ci-check-failures. To fix the dependency problem, please look at the bazel file changes here for example: https://github.com/oppia/oppia-android/pull/5446/files |
I am building my oppia-android bazel in my ubuntu and i am getting this issue !! please help me to resolve it |
@theMr17, PTAL. |
@whyash8 This issue has been encountered before. Please refer to the latter part of the comment for more details: #4886 (comment). |
Hi @whyash8, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. |
I have successfully build the bazel . But due to my university exams i will be able to continue after 3 days. Sorry for the inactivity. |
Hi @whyash8, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. |
I am working on it! |
Hi @whyash8, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. |
Ok! |
can you help where to add my new utility in the app/ build.BAZEL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@whyash8, the path to the util should be added to the app/BUILD.bazel file in the same way as the ProtoExtraMatcher
in the example PR I linked earlier. Make sure the path is added at a position in alphabetical order, or the lint checks will fail.
UserTypeAnswer.values().toList(), | ||
{it}, | ||
{it.isValid()} | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to name the variable(it) to improve readability. Ditto the other files in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i have made the changes
{it}, | ||
{it.isValid()} | ||
) | ||
observableList+=filteredUserTypes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: reformat. There needs to be spaces around the +=. Ditto the other files in this PR. Use Ctrl+Alt+L
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i have made the changes
added the utility path same as the file you told and i also did add the path to analytics/build.BAZEL but i am getting this error please take a look in the latest commit . /home/yash-sharma/OPENSOURCE/oppia-android/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/BUILD.bazel:41:19: no such package 'utility/src/main/java/org/oppia/android/util/enumfilter/EnumFilterUtil.kt': BUILD file not found in any of the following directories. Add a BUILD file to a directory to mark it as a package.
|
making a seperate bazel file is working and it also adds to the analytics build.bazel file and its building the bazel without any errors . |
Sounds good @whyash8 |
So can I do that instead @adhiamboperes |
the isValid () function is being used in the enumfilter . I am successfully building the bazel by running bazel build//:oppia with the latest commit and i have done all the changes which were asked . please help to proceed further . thanks @adhiamboperes |
APK & AAB differences analysisNote that this is a summarized snapshot. See the CI artifacts for detailed differences. DevExpand to see flavor specificsUniversal APKAPK file size: 19 MiB (old), 19 MiB (new), 6344 bytes (Added) APK download size (estimated): 17 MiB (old), 17 MiB (new), 3678 bytes (Added) Method count: 259592 (old), 259759 (new), 167 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 6816 (old), 6816 (new), 0 (No change)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 18 MiB (old), 18 MiB (new), 6344 bytes (Added) Configuration hdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 49 KiB (old), 49 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 45 KiB (old), 45 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 86 KiB (old), 86 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 57 KiB (old), 57 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change) AlphaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 52 bytes (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 247 bytes (Removed) Method count: 116052 (old), 116054 (new), 2 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5784 (old), 5784 (new), 0 (No change)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 52 bytes (Added) Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) BetaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 84 bytes (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 1673 bytes (Removed) Method count: 116058 (old), 116060 (new), 2 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5784 (old), 5784 (new), 0 (No change)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 84 bytes (Added) Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) GaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 232 bytes (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 2291 bytes (Removed) Method count: 116058 (old), 116060 (new), 2 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5784 (old), 5784 (new), 0 (No change)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 228 bytes (Added) Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) |
APK & AAB differences analysisNote that this is a summarized snapshot. See the CI artifacts for detailed differences. DevExpand to see flavor specificsUniversal APKAPK file size: 19 MiB (old), 19 MiB (new), 5812 bytes (Added) APK download size (estimated): 17 MiB (old), 17 MiB (new), 3661 bytes (Added) Method count: 259592 (old), 259759 (new), 167 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 6816 (old), 6816 (new), 0 (No change)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 18 MiB (old), 18 MiB (new), 5808 bytes (Added) Configuration hdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 49 KiB (old), 49 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 45 KiB (old), 45 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 86 KiB (old), 86 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 57 KiB (old), 57 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change) AlphaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 484 bytes (Removed) APK download size (estimated): 10 MiB (old), 10 MiB (new), 287 bytes (Removed) Method count: 116052 (old), 116054 (new), 2 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5784 (old), 5784 (new), 0 (No change)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 484 bytes (Removed) Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) BetaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 452 bytes (Removed) APK download size (estimated): 10 MiB (old), 10 MiB (new), 1776 bytes (Removed) Method count: 116058 (old), 116060 (new), 2 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5784 (old), 5784 (new), 0 (No change)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 452 bytes (Removed) Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) GaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 308 bytes (Removed) APK download size (estimated): 10 MiB (old), 10 MiB (new), 2384 bytes (Removed) Method count: 116058 (old), 116060 (new), 2 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5784 (old), 5784 (new), 0 (No change)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 308 bytes (Removed) Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) |
please help @adhiamboperes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @whyash8!
Great job setting up bazel and getting it to work.
I have left some suggestions to clean up the PR.
Additionally, to solve the codeowners check failure, please open the CODEOWNERS
file and add the path to the util under # Utilities that are primarily used for frontend/UI purposes.
Please maintain alphabetical order of list.
Finally, please don't resolve the review comments. I will resolve them once I verify that everything works.
utility/src/main/java/org/oppia/android/util/enumfilter/BUILD.bazel
Outdated
Show resolved
Hide resolved
utility/src/main/java/org/oppia/android/util/enumfilter/BUILD.bazel
Outdated
Show resolved
Hide resolved
utility/src/main/java/org/oppia/android/util/enumfilter/EnumFilterUtil.kt
Outdated
Show resolved
Hide resolved
utility/src/main/java/org/oppia/android/util/enumfilter/BUILD.bazel
Outdated
Show resolved
Hide resolved
utility/src/main/java/org/oppia/android/util/enumfilter/BUILD.bazel
Outdated
Show resolved
Hide resolved
domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/BUILD.bazel
Outdated
Show resolved
Hide resolved
…bazel Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com>
…lterUtil.kt Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com>
…ing it to CODEOWNER
Hi mentors, I have made the requested changes. Could you please take a look and let me know if any further adjustments are needed? Thank you! @adhiamboperes |
APK & AAB differences analysisNote that this is a summarized snapshot. See the CI artifacts for detailed differences. DevExpand to see flavor specificsUniversal APKAPK file size: 19 MiB (old), 19 MiB (new), 1724 bytes (Added) APK download size (estimated): 17 MiB (old), 17 MiB (new), 1172 bytes (Added) Method count: 259592 (old), 259593 (new), 1 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 6816 (old), 6816 (new), 0 (No change)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 18 MiB (old), 18 MiB (new), 1724 bytes (Added) Configuration hdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 49 KiB (old), 49 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 45 KiB (old), 45 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 86 KiB (old), 86 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 57 KiB (old), 57 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change) AlphaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 84 bytes (Added) APK download size (estimated): 10 MiB (old), 10 MiB (new), 277 bytes (Removed) Method count: 116052 (old), 116052 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5784 (old), 5784 (new), 0 (No change)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 80 bytes (Added) Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) BetaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 32 bytes (Removed) APK download size (estimated): 10 MiB (old), 10 MiB (new), 1442 bytes (Removed) Method count: 116058 (old), 116058 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5784 (old), 5784 (new), 0 (No change)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 32 bytes (Removed) Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) GaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 32 bytes (Removed) APK download size (estimated): 10 MiB (old), 10 MiB (new), 2767 bytes (Removed) Method count: 116058 (old), 116058 (new), 0 (No change) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5784 (old), 5784 (new), 0 (No change)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 28 bytes (Removed) Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) |
why the comment coverage is failing ? |
Explanation
Fixes: #3146
This PR introduces a new utility function filterByEnumCondition to standardize filtering of enums across various parts of the oppia-android codebase. This utility function allows filtering of collections based on a condition applied to enum values.
This PR introduces a new utility function filterByEnumCondition to standardize filtering of enums across various parts of the oppia-android codebase. This utility function allows filtering of collections based on a condition applied to enum values.
Key Changes:
Added Utility Function:
filterByEnumCondition: A generic function to filter a collection based on a condition applied to an enum extracted from each item in the collection.
Updated Existing Code:
Refactored code in getLeastRecentMetricLogIndex, getLeastRecentMediumPriorityEventIndex, and other methods to utilize the new filterByEnumCondition function.
Updated the calculation of completedChapterCount and inProgressChapterCount using the new utility function.
Essential Checklist