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 part of #1833: Prepare to rename package to org.oppia.android (part 1/2) #1879

Merged

Conversation

BenHenning
Copy link
Member

Fixes part of #1833.

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 reviews.

THIS WILL BREAK THE CODEBASE WHEN CHECKED IN. I'll be sending out an email shortly explaining the situation to oppia-android-dev@.

#1876 (or a similar PR) will follow up with the actual import changes to fix everything.

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.
@BenHenning
Copy link
Member Author

@rt4914 @anandwana001 PTAL. Note that checks will fail for this, which means that the PR will need to be force merged.

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, thanks @BenHenning
Just concern with two points:

  1. What about the model package

Screenshot 2020-09-24 at 23 08 35

2. `org.oppia.android.util` is fine or it should be `org.oppia.android.utility`? although util looks fine, but just to confirm as all other are same as the package name

@anandwana001 anandwana001 assigned BenHenning and rt4914 and unassigned rt4914 and anandwana001 Sep 24, 2020
@BenHenning
Copy link
Member Author

BenHenning commented Sep 24, 2020

@anandwana001 there are no directory structure changes needed in model. The actual package references should be updated in #1876 (see example.proto in that PR--I can't link to it since the PR is so large).

Does this address your question?

@anandwana001
Copy link
Contributor

@anandwana001 there are no directory structure changes needed in model. The actual package references should be updated in #1876 (see example.proto in that PR--I can't link to it since the PR is so large).

Does this address your question?

Yes, I got it.

@BenHenning
Copy link
Member Author

Thanks.

NB: The Gradle tests are passing due to Gradle not actually enforcing that file packages match their directory structure. Bazel is stricter, so it's breaking as expected.

@BenHenning
Copy link
Member Author

@rt4914 I will block submission on your review.

@BenHenning BenHenning removed their assignment Sep 24, 2020
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@rt4914 rt4914 assigned BenHenning and unassigned rt4914 Sep 24, 2020
@BenHenning
Copy link
Member Author

Thanks! I will submit this only after #1876 is approved.

@BenHenning
Copy link
Member Author

Email is ready to go out, and will be sent immediately after merging. I have updated branch protections to require all PRs are up-to-date with develop, thereby preventing people from submitting changes after this PR goes in (since it breaks the Bazel check). This isn't a super robust way to stop changes, but it should well enough for the next ~30 minutes.

Force merging this PR despite failing checks. Checks will be passing again in #1876.

@BenHenning BenHenning merged commit a4910c8 into develop Sep 24, 2020
@BenHenning BenHenning deleted the move-files-to-prepare-for-new-app-package-structure branch September 24, 2020 20:47
@BenHenning BenHenning added the PR: Cherrypick requested Indicates that a PR is being requested for being cherrypicked into the ongoing release branch. label Sep 25, 2020
BenHenning added a commit that referenced this pull request Sep 25, 2020
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.
@BenHenning BenHenning added the PR: Cherrypick completed Indicates a cherrypick request was approved & completed for a PR. label Sep 25, 2020
prayutsu pushed a commit to prayutsu/oppia-android that referenced this pull request Sep 25, 2020
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants