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

Remove :voiceover_java_proto_lite from the list of exports in the model library #2768

Closed
fsharpasharp opened this issue Feb 23, 2021 · 5 comments · Fixed by #2999
Closed
Assignees
Labels
good first issue This item is good for new contributors to make their pull request. Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@fsharpasharp
Copy link
Contributor

This task is part of the build system migration from Gradle to Bazel. A prerequesite before working on this issue is that you are able to build Oppia using Bazel. Check out Oppia Bazel guide for Bazel terminology and more on what Bazel does.

1. Remove the export

In

/oppia-android/model/BUILD.bazel

Remove :voiceover_java_proto_lite from the list of exports in the model library

android_library(
    name = "model",
    visibility = ["//visibility:public"],
    exports = [
        ... # <- Remove it from this list.
    ],
)

2. Add public visibility to the java_lite_proto_library

Add

visibility = ["//visibility:public"],

to the java_lite_proto_library that has the name voiceover_java_proto_lite.

3. Build the app using Bazel and run the tests

Libraries that are indirectly depending on voiceover_java_proto_lite will now fail to build. Try to build the entire app with Bazel and add the deps dependency

//model:voiceover_java_proto_lite,

to all the libraries that are now failing to build. You will have to look around for BUILD.bazel files that have libraries that require this replaced dependency.

These libraries will already have the //model dependency and since we removed the :voiceover_java_proto_lite from the model library itself, they will need to add the newly available public library directly as a dependency. (Not all libraries with the //model dependency may require the newly available library)

Once the app builds and all tests pass, you're done!

@Karanjot-singh
Copy link
Contributor

I would like to work on this issue @prayutsu

@Karanjot-singh
Copy link
Contributor

Karanjot-singh commented Mar 24, 2021

@prayutsu Please address the following queries as well

  1. After completion of task 1 and 2 as mentioned in the issue above, The build was successful. Where am I supposed to add the deps dependency //model:voiceover_java_proto_lite (as per task 3)?

  2. How do I lint check my BUILD.bazel file?

  3. The following unit Tests failed in the old PR Could you help me with these so that I don't make the same mistakes in the new PR
    image
    image

@fsharpasharp
Copy link
Contributor Author

@Karanjot-singh, if you link a PR, I can answer all these questions for you.

@Karanjot-singh
Copy link
Contributor

Thank you @fsharpasharp I am working on a new PR... I will link it soon

@Karanjot-singh
Copy link
Contributor

@Karanjot-singh, if you link a PR, I can answer all these questions for you.

Kindly refer #2999

rt4914 pushed a commit that referenced this issue Mar 28, 2021
… in the model library (#2999)

* Remove voiceover_java_proto_lite from model exports

* visibility removed in java_proto_lite & extra whitespace removed
@BenHenning BenHenning added the Z-ibt Temporary label for Ben to keep track of issues he's triaged. label Sep 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue This item is good for new contributors to make their pull request. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Projects
4 participants