-
Notifications
You must be signed in to change notification settings - Fork 528
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 #1833: Rename package to org.oppia.android (part 2/2) #1876
Conversation
actual package or code references are updated yet.
This includes introducing a new developer Firebase project. It also fixes references to Android resource R file imports & databinding since they both exist at the package level rather than the app module level.
This required some changes to test setup/arrangement in Bazel, and some existing issues needed to be fixed (missing dependencies for work manager, switching multidex, one renamed test, incorrect module references in tests, and files now referencing resources needing to be moved). This includes some Bazel file reformatting that happened automatically by my Bazel file plugin.
Note that this intentionally keeps the merge artifacts in since they will be resolved en masse with find-and-replace commands that will undergo manual verification. Conflicts: app/src/main/java/org/oppia/android/app/administratorcontrols/AdministratorControlsViewModel.kt app/src/main/java/org/oppia/android/app/administratorcontrols/administratorcontrolsitemviewmodel/AdministratorControlsDownloadPermissionsViewModel.kt app/src/main/java/org/oppia/android/app/application/ApplicationInjector.kt app/src/main/java/org/oppia/android/app/completedstorylist/CompletedStoryListViewModel.kt app/src/main/java/org/oppia/android/app/drawer/NavigationDrawerFragmentPresenter.kt app/src/main/java/org/oppia/android/app/home/HomeFragmentPresenter.kt app/src/main/java/org/oppia/android/app/home/recentlyplayed/RecentlyPlayedFragmentPresenter.kt app/src/main/java/org/oppia/android/app/ongoingtopiclist/OngoingTopicListViewModel.kt app/src/main/java/org/oppia/android/app/options/OptionControlsViewModel.kt app/src/main/java/org/oppia/android/app/options/OptionsFragmentPresenter.kt app/src/main/java/org/oppia/android/app/player/audio/AudioFragmentPresenter.kt app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivityPresenter.kt app/src/main/java/org/oppia/android/app/player/exploration/ExplorationManagerFragmentPresenter.kt app/src/main/java/org/oppia/android/app/player/exploration/HintsAndSolutionExplorationManagerFragmentPresenter.kt app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt app/src/main/java/org/oppia/android/app/profile/AddProfileActivityPresenter.kt app/src/main/java/org/oppia/android/app/profile/AdminPinActivityPresenter.kt app/src/main/java/org/oppia/android/app/profile/PinPasswordActivityPresenter.kt app/src/main/java/org/oppia/android/app/profile/PinPasswordViewModel.kt app/src/main/java/org/oppia/android/app/profile/ProfileChooserFragmentPresenter.kt app/src/main/java/org/oppia/android/app/profile/ProfileChooserViewModel.kt app/src/main/java/org/oppia/android/app/profile/ResetPinDialogFragmentPresenter.kt app/src/main/java/org/oppia/android/app/profileprogress/ProfilePictureActivityPresenter.kt app/src/main/java/org/oppia/android/app/profileprogress/ProfileProgressViewModel.kt app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditActivityPresenter.kt app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditViewModel.kt app/src/main/java/org/oppia/android/app/settings/profile/ProfileListViewModel.kt app/src/main/java/org/oppia/android/app/settings/profile/ProfileRenameActivityPresenter.kt app/src/main/java/org/oppia/android/app/settings/profile/ProfileResetPinActivityPresenter.kt app/src/main/java/org/oppia/android/app/splash/SplashActivityPresenter.kt app/src/main/java/org/oppia/android/app/story/StoryViewModel.kt app/src/main/java/org/oppia/android/app/topic/TopicViewModel.kt app/src/main/java/org/oppia/android/app/topic/info/TopicInfoFragmentPresenter.kt app/src/main/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentPresenter.kt app/src/main/java/org/oppia/android/app/topic/practice/TopicPracticeViewModel.kt app/src/main/java/org/oppia/android/app/topic/questionplayer/HintsAndSolutionQuestionManagerFragmentPresenter.kt app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragmentPresenter.kt app/src/main/java/org/oppia/android/app/topic/revision/TopicRevisionViewModel.kt app/src/main/java/org/oppia/android/app/walkthrough/end/WalkthroughFinalFragmentPresenter.kt app/src/main/java/org/oppia/android/app/walkthrough/welcome/WalkthroughWelcomeFragmentPresenter.kt app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt app/src/sharedTest/java/org/oppia/android/app/profile/AddProfileActivityTest.kt app/src/sharedTest/java/org/oppia/android/app/profile/AdminAuthActivityTest.kt app/src/sharedTest/java/org/oppia/android/app/profile/AdminPinActivityTest.kt app/src/sharedTest/java/org/oppia/android/app/profile/PinPasswordActivityTest.kt app/src/sharedTest/java/org/oppia/android/app/profile/ProfileChooserFragmentTest.kt app/src/sharedTest/java/org/oppia/android/app/settings/profile/ProfileEditActivityTest.kt app/src/sharedTest/java/org/oppia/android/app/settings/profile/ProfileRenameActivityTest.kt app/src/sharedTest/java/org/oppia/android/app/settings/profile/ProfileResetPinActivityTest.kt app/src/test/java/org/oppia/android/app/home/HomeActivityLocalTest.kt app/src/test/java/org/oppia/android/app/player/exploration/ExplorationActivityLocalTest.kt app/src/test/java/org/oppia/android/app/player/state/StateFragmentLocalTest.kt app/src/test/java/org/oppia/android/app/testing/CompletedStoryListSpanTest.kt app/src/test/java/org/oppia/android/app/testing/OngoingTopicListSpanTest.kt app/src/test/java/org/oppia/android/app/testing/ProfileChooserSpanTest.kt app/src/test/java/org/oppia/android/app/testing/ProfileProgressSpanCount.kt app/src/test/java/org/oppia/android/app/testing/TopicRevisionSpanTest.kt app/src/test/java/org/oppia/android/app/testing/options/AppLanguageFragmentTest.kt app/src/test/java/org/oppia/android/app/testing/options/DefaultAudioFragmentTest.kt app/src/test/java/org/oppia/android/app/testing/options/OptionsFragmentTest.kt app/src/test/java/org/oppia/android/app/testing/options/ReadingTextSizeFragmentTest.kt app/src/test/java/org/oppia/android/app/testing/player/split/PlayerSplitScreenTesting.kt app/src/test/java/org/oppia/android/app/testing/player/state/StateFragmentAccessibilityTest.kt app/src/test/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityLocalTest.kt data/src/test/java/org/oppia/android/data/persistence/PersistentCacheStoreTest.kt domain/src/main/java/org/oppia/android/domain/audio/CellularAudioDialogController.kt domain/src/main/java/org/oppia/android/domain/exploration/ExplorationDataController.kt domain/src/main/java/org/oppia/android/domain/exploration/ExplorationProgressController.kt domain/src/main/java/org/oppia/android/domain/onboarding/AppStartupStateController.kt domain/src/main/java/org/oppia/android/domain/profile/ProfileManagementController.kt domain/src/main/java/org/oppia/android/domain/question/QuestionAssessmentProgressController.kt domain/src/main/java/org/oppia/android/domain/question/QuestionTrainingController.kt domain/src/main/java/org/oppia/android/domain/topic/StoryProgressController.kt domain/src/main/java/org/oppia/android/domain/topic/TopicController.kt domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt domain/src/test/java/org/oppia/android/domain/audio/CellularAudioDialogControllerTest.kt domain/src/test/java/org/oppia/android/domain/exploration/ExplorationDataControllerTest.kt domain/src/test/java/org/oppia/android/domain/exploration/ExplorationProgressControllerTest.kt domain/src/test/java/org/oppia/android/domain/onboarding/AppStartupStateControllerTest.kt domain/src/test/java/org/oppia/android/domain/oppialogger/analytics/AnalyticsControllerTest.kt domain/src/test/java/org/oppia/android/domain/oppialogger/exceptions/ExceptionsControllerTest.kt domain/src/test/java/org/oppia/android/domain/oppialogger/exceptions/UncaughtExceptionLoggerStartupListenerTest.kt domain/src/test/java/org/oppia/android/domain/profile/ProfileManagementControllerTest.kt domain/src/test/java/org/oppia/android/domain/question/QuestionAssessmentProgressControllerTest.kt domain/src/test/java/org/oppia/android/domain/question/QuestionTrainingControllerTest.kt domain/src/test/java/org/oppia/android/domain/topic/StoryProgressControllerTest.kt domain/src/test/java/org/oppia/android/domain/topic/StoryProgressTestHelperTest.kt domain/src/test/java/org/oppia/android/domain/topic/TopicControllerTest.kt domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt testing/src/main/java/org/oppia/android/testing/profile/ProfileTestHelper.kt testing/src/test/java/org/oppia/android/testing/profile/ProfileTestHelperTest.kt utility/src/test/java/org/oppia/android/util/data/DataProvidersTest.kt
This won't compile due to duplicate imports which requires a ktlint clean-up. That will happen in a subsequent commit since it will likely pick up unrelated changes.
Fix new references to R and databinding. Move EditTextInputAction to correct package.
@rt4914 @anandwana001 @vinitamurthi I'm concerned about this PR. GitHub is not recognizing these are moved files, which means it's going to destroy history (not to mention, this PR is impossibly long to review). One alternative that we have is that I could split this into two PRs:
While that will reduce the number of lines to review by an order of magnitude (and make it trivial since the lines changed are mostly just imports), it means that the repository is fully broken between (1) & (2). We also have to create (2) after (1) is merged otherwise we run into the same problem that we have now. Any thoughts or suggestions? |
@BenHenning We can decide a particular time and during that time we can finish part 1 and 2 so that even if it breaks it will break for a very less time. I am thinking of you time because most of our contributors are from India. |
This moves all Kotlin source files to be under an org.oppia.android structure, but does not actually change references. This is done in two steps to try and preserve history, and to simplify the code review. THIS WILL BREAK THE CODEBASE WHEN CHECKED IN.
Sounds good--thanks @rt4914. As discussed during the team meeting, we'll do this today & split it into two PRs. |
…to introduce-new-package-structure Conflicts: app/src/main/java/org/oppia/android/app/application/ApplicationComponent.kt domain/src/main/java/org/oppia/android/domain/classify/rules/RuleQualifiers.kt
Aha, it looks like changing the base does work. I'll send both PRs into review now so that we can make sure all approvals are in order before attempting submission. |
@rt4914 @anandwana001 PTAL. |
@rt4914 regarding the empty folders, that's just a local view that you have. Directories aren't tracked by git, just files. I suspect we may all have some empty folders after updating to the changes in this PR, and I'm not sure that I can do anything about that. Otherwise, I think both of your comments have been addressed. PTAL and verify. |
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.
LGTM, thanks.
Capitalize TODO comment for consistency with other TODO comments.
Thanks @rt4914. @anandwana001 I'm going to go ahead and merge this to try and get it in before SLoP folks start ramping up. Please let me know afterwards if you have any concerns with the PR and I'll be happy to address them in a subsequent PR. |
Sounds good. Thanks for wrapping up this quickly. |
…to introduce-new-package-structure
#1879 has been merged & the history of this branch has been updated. Diff looks clean against develop, so just waiting for CI checks to pass before merging. Hopefully the history is retained post-merge. |
Looks like history wasn't retained: https://github.com/oppia/oppia-android/commits/develop/app/src/main/java/org/oppia/android/app/activity/ActivityComponent.kt :(. This is a known issue for GitHub: isaacs/github#900, so it will possibly be fixed in the future. I think the order we did this will be compatible with
I might try the suggested Chrome extension after this PR is submitted to see if it's effective. |
CI checks are passing. Going ahead and merging to fix develop. |
Disabled the 'require branch to be up-to-date' check. Just sent out the 'all-clear' email. |
"Follow for Github" extension does not seem to work (see reviews--this might have to do with some GitHub redesigning). The Refined GitHub extension seems to work, though: I probably won't be using the extension yet, but it's a nice option to have. |
Conflicts when trying to CP into the release branch:
|
Due to the significant numbers of conflicts, this needs to be manually CP'd so that it can undergo code review. |
…#1876) * Move all source files to new org/oppia/android directory structure. No actual package or code references are updated yet. * Update Kotlin & XML references to match new package structure. * Migrate codepaths for Bazel builds to new package structure. * Fix package references in Gradle files for new package structure. * Update package & pathing references to new structure for proto files. * Rename Android package to org.oppia.android. This includes introducing a new developer Firebase project. It also fixes references to Android resource R file imports & databinding since they both exist at the package level rather than the app module level. * Fix Bazel builds for new package structure. This required some changes to test setup/arrangement in Bazel, and some existing issues needed to be fixed (missing dependencies for work manager, switching multidex, one renamed test, incorrect module references in tests, and files now referencing resources needing to be moved). This includes some Bazel file reformatting that happened automatically by my Bazel file plugin. * Resolve merge conflicts. This won't compile due to duplicate imports which requires a ktlint clean-up. That will happen in a subsequent commit since it will likely pick up unrelated changes. * Remove some merge conflict markers that were missed. * Remove redundant commits & other ktlint updates. * Fix incorrect references to new testing package location. * Post-testing module cleanups via ktlint. * More post-merge fixes. Fix new references to R and databinding. Move EditTextInputAction to correct package. * Move some missed files that only caused breakages in Bazel. * Move all files to their correct new location. This moves all Kotlin source files to be under an org.oppia.android structure, but does not actually change references. This is done in two steps to try and preserve history, and to simplify the code review. THIS WILL BREAK THE CODEBASE WHEN CHECKED IN. * Fix missed files from the migration after merging. * Disable Firebase data collection. * Update BUILD.bazel Capitalize TODO comment for consistency with other TODO comments.
Fix #1833.
This PR renames the Android package to org.oppia.android. A few things to note:
Note that this is a continuation of #1879. This PR will be merged immediately after #1879 goes in & CI checks are passing (which means the codebase will be intermittently broken for a short period of time--submissions will be disabled during that point).