-
Notifications
You must be signed in to change notification settings - Fork 541
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
Remove genrules once Gradle has been removed #1617
Labels
bug
End user-perceivable behaviors which are not desirable.
Impact: Low
Low perceived user impact (e.g. edge cases).
Issue: Needs Clarification
Indicates that an issue needs more detail in order to be able to be acted upon.
Z-ibt
Temporary label for Ben to keep track of issues he's triaged.
Comments
6 tasks
BenHenning
added a commit
that referenced
this issue
Mar 25, 2022
…4045) ## Explanation Fix part of #4044 Fix part of #1617 This PR makes #1543 obsolete once merged. Originally copied from #2173 when it was in proof-of-concept form This PR replaces the proto formatting mechanism (or, rather, makes it obsolete) by leveraging a feature of Bazel's proto library that allows the import prefix to be stripped. This ensures that the generated protos are treated as though they're in the same directory (which is how they were set up before per Gradle's requirement). The main benefit of this slightly cleaner approach is it allows the textproto conversion pipeline to work for protos that import other protos (otherwise the path doesn't match up, and it ends up with a Bazel error). This required introducing a new proto library to ensure the prefix is properly stripped, and thus this PR introduces a regex check + test to force this check. As part of introducing this check, file matching & exemption checking now allows for more flexible regex (via checks). ## 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 only changes build infrastructure & has no direct impact on user interfaces. Commit history: * Copy proto-based changes from #2173. * Post-merge fix. * Add regex check, docs, and resolve TODOs. This also changes regex handling in the check to be more generic for better flexibility when matching files. * Lint fix. * Fix broken build. * Post-merge fix.
6 tasks
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
bug
End user-perceivable behaviors which are not desirable.
Impact: Low
Low perceived user impact (e.g. edge cases).
Issue: Needs Clarification
Indicates that an issue needs more detail in order to be able to be acted upon.
Z-ibt
Temporary label for Ben to keep track of issues he's triaged.
The genrules in app module's BUILD file are necessary only for building Oppia Android in both Gradle and Bazel. Similarly, the custom macro in the model module (
format_import_proto_library.bzl
) will need to be removed once Gradle is out of the picture. For both genrules, removing them will require changing some import statements (Ex.import org.oppia.app.R
->import org.oppia.app.vm.R
).The text was updated successfully, but these errors were encountered: