-
Notifications
You must be signed in to change notification settings - Fork 499
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 #3324: Created BUILD.bazel in oppialogger inside domain module #3739
Fix #3324: Created BUILD.bazel in oppialogger inside domain module #3739
Conversation
Hi @deepanshu731. My apologies for the delay--I'll need to take a look at this tomorrow. |
Hi--sorry again about the delay. I still plan to look at this later in the week; I've just had some project emergencies to deal with in the meantime. Thanks for your patience. |
Its okay @BenHenning |
Hi @deepanshu731, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 3 days, it will be automatically closed so that others can take up the issue. |
Hi @deepanshu731, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 3 days, it will be automatically closed so that others can take up the issue. |
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.
Thanks @deepanshu731! This looks really solid! Just had a few comments.
Sorry for the extended delay--I plan to prioritize this PR moving forward.
domain/src/main/java/org/oppia/android/domain/oppialogger/BUILD.bazel
Outdated
Show resolved
Hide resolved
domain/src/main/java/org/oppia/android/domain/oppialogger/exceptions/BUILD.bazel
Outdated
Show resolved
Hide resolved
@BenHenning these tests are failing after updating from develop (Did to resolve merge conflict) maybe some realted to databinding are failing where i didnt change anything , Can you help me out with this ? |
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.
Thanks @deepanshu731. Had just a few more comments, but this PR is looking good to me overall.
@BenHenning these tests are failing after updating from develop (Did to resolve merge conflict) maybe some realted to databinding are failing where i didnt change anything , Can you help me out with this ?
This is a CI flake (see #3759). Restarting should fix it, but given that I requested a couple more changes you could probably just push a new commit with those fixes & CI should pass on the re-run.
domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/BUILD.bazel
Outdated
Show resolved
Hide resolved
domain/src/main/java/org/oppia/android/domain/oppialogger/exceptions/BUILD.bazel
Outdated
Show resolved
Hide resolved
…ytics/BUILD.bazel Co-authored-by: Ben Henning <henning.benmax@gmail.com>
…ptions/BUILD.bazel Co-authored-by: Ben Henning <henning.benmax@gmail.com>
Thanks @BenHenning PTAL. |
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.
Thanks @deepanshu731. PR LGTM! Thanks so much for your patience with the slow reviews.
Fixed #3324
Created a BUILD.bazel file in oppialogger package inside domain.
Checklist