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.

Sign up for GitHub

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

Jump to bottom

Fix #3937: Ensure ViewEventLogsViewModel builds for alpha builds #3957

Merged
merged 2 commits into from
Oct 22, 2021
Merged

Fix #3937: Ensure ViewEventLogsViewModel builds for alpha builds #3957

merged 2 commits into from
Oct 22, 2021

Conversation

BenHenning
Copy link
Sponsor Member

@BenHenning BenHenning commented Oct 21, 2021 •
edited
Loading

Explanation

Fix #3937

This updates DebugEventLogger's Bazel build such that it's still available to depend on for alpha production builds. This is sort of a hacky workaround needed to make ViewEventLogsViewModel compile since view models can't be conditionally excluded in builds yet. Post #59 we'll be able to exclude debug-only view models like ViewEventLogsViewModel in prod builds.

Note that there's not a reasonable way to test this today besides manually building. Also, CI doesn't catch this since our alpha build is still set up to use debug builds (which we can't move away until probably after #1720 is addressed. You can repro this locally by following this excerpt from the release process & then trying to build a debug/alpha version of the app:

  • In ApplicationComponent.kt, make the following replacements (which may require imports):
    • Remove modules:
      1. DeveloperOptionsStarterModule
      2. DebugLogReportingModule
      3. NetworkConnectionUtilDebugModule
      4. HintsAndSolutionDebugModule
    • Add modules:
      1. LogReportingModule
      2. NetworkConnectionUtilProdModule
      3. HintsAndSolutionProdModule
    • The above disables the developer options menu and developer-only functionality
  • In app/BUILD.bazel
    • In view_models, in: //utility/src/main/java/org/oppia/android/util/logging/firebase:debug_module, replace “debug_module” with prod_module
    • In “app”, in //utility/src/main/java/org/oppia/android/util/networking:debug_module, replace debug_module with prod_module
    • This sets up the build dependencies correctly for the changes made above in ApplicationComponent.kt

Essential Checklist

  • 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: ...".)
  • Any changes to scripts/assets files have their rationale included in the PR explanation.
  • The PR follows the style guide.
  • The PR does not contain any unnecessary code changes from Android Studio (reference).
  • The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
  • The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

N/A -- not a user-facing change.

@BenHenning BenHenning added this to the Alpha MR3 milestone Oct 21, 2021
@BenHenning BenHenning added the PR: Cherrypick requested Indicates that a PR is being requested for being cherrypicked into the ongoing release branch. label Oct 21, 2021
@BenHenning BenHenning marked this pull request as ready for review October 21, 2021 06:53
@BenHenning
Copy link
Sponsor Member Author

@anandwana001 PTAL.

anandwana001
anandwana001 approved these changes Oct 22, 2021
View reviewed changes
Copy link
Contributor

@anandwana001 anandwana001 left a comment

Choose a reason for hiding this comment

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

LGTM as it is building successfully on local.

Screenshot 2021-10-22 at 08 21 42

@oppiabot
Copy link

oppiabot bot commented Oct 22, 2021

Unassigning @anandwana001 since they have already approved the PR.

@oppiabot oppiabot bot added the PR: LGTM label Oct 22, 2021
@oppiabot
Copy link

oppiabot bot commented Oct 22, 2021

Hi @BenHenning, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks!

@BenHenning
Copy link
Sponsor Member Author

Thanks @anandwana001! Merging since there are no open threads, and the PR has necessary approvals.

@BenHenning BenHenning merged commit 056c628 into develop Oct 22, 2021
@BenHenning BenHenning deleted the ensure-view-event-logs-view-model-builds-for-prod-app branch October 22, 2021 03:26
BenHenning added a commit that referenced this pull request Oct 27, 2021
@BenHenning
Fix #3937: Ensure ViewEventLogsViewModel builds for alpha builds (#3957)
65f9c0d 
* Ensure ViewEventLogsViewModel builds for alpha.

* BUILD file lint fix.
This was referenced Oct 27, 2021
Closed
Merged
BenHenning added a commit that referenced this pull request Oct 29, 2021
@BenHenning
[RunAllTests] Fix part of #3974: Cherry-pick MR3 release blocker fixes (
0506c5d 
#3975)

* Fix #3946: Fix solution automatically revealing after first hint reveal (#3955)

* Fix solution auto showing after hint reveal.

See #3946 & PR for specifics.

* Add translations for new hint/solution.

* Lint fixes.

* Add TODO.

* Fix Gradle-variant tests.

* Fix #3937: Ensure ViewEventLogsViewModel builds for alpha builds (#3957)

* Ensure ViewEventLogsViewModel builds for alpha.

* BUILD file lint fix.

* Fix #3939 & #3938: Fix KitKat crash & SVG rendering issues (#3963)

* Fix KitKat crash when opening Help menu.

* Fix SVG rendering on SDKs 19-23 (incl).

* Add regex check to prohibit Delegates.

* Lint fixes.

* Add exemption for regex script test.

* Update file_content_validation_checks.textproto

Grammar fix in error.

* Update RegexPatternValidationCheckTest.kt

Copy grammar fix to test copy of error.

* Update version.bzl (#3964)

Bump version codes for another RC of release-0.6.

* Embed proguard.map in optimized AAB builds. (#3973)
@BenHenning BenHenning added the PR: Cherrypick completed Indicates a cherrypick request was approved & completed for a PR. label Oct 30, 2021
@BenHenning
Copy link
Sponsor Member Author

This is verified as working on the release-0.6 build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anandwana001 anandwana001 anandwana001 approved these changes

Assignees

@BenHenning BenHenning

Labels
PR: Cherrypick completed Indicates a cherrypick request was approved & completed for a PR. PR: Cherrypick requested Indicates that a PR is being requested for being cherrypicked into the ongoing release branch. PR: LGTM
Projects
None yet
Milestone
0.6 Release Blockers (Alpha MR3)
Development

Successfully merging this pull request may close these issues.

ViewEventLogsViewModel depends on a debug-only dependency
2 participants
@BenHenning @anandwana001