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

Add proto config files for feature and required class. #4869

Closed

Conversation

KevinGitonga
Copy link
Contributor

@KevinGitonga KevinGitonga commented Feb 6, 2023

Fix #4865
When this PR is merged it should enable developers to easily access ProfileId which will be wrapped wrapped into all Activities Intents.

Essential Checklist

  • 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: ...".)
  • Any changes to scripts/assets files have their rationale included in the PR explanation.
  • The PR follows the style guide.
  • The PR does not contain any unnecessary code changes from Android Studio (reference).
  • The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
  • The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

If your PR includes UI-related changes, then:

  • Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes
  • For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see RTL guide)
  • Add a video showing the full UX flow with a screen reader enabled (see accessibility guide)
  • Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

@KevinGitonga
Copy link
Contributor Author

Hi @BenHenning, I have added the core classes and refactored the implementation to to enable provision of ProfileId to multiple activities intents. Kindly check on the implementation and let me know if i am in the right direction on this. Please note that it works as expected with gradle but not via bazel. This is even after adding bazel config for the decorator class similarly to screen name decorator.Thanks.

Copy link
Member

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

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

Thanks @KevinGitonga. Overall I think the approach looks reasonable, though I had one suggestion to simplify the decorator a bit.

That being said, what precisely isn't working for Bazel? The CI checks are passing, so it seems like everything is working correctly. Am I missing something?

@BenHenning BenHenning assigned KevinGitonga and unassigned BenHenning Feb 8, 2023
@KevinGitonga KevinGitonga marked this pull request as ready for review February 9, 2023 16:41
@KevinGitonga KevinGitonga requested a review from rt4914 as a code owner February 9, 2023 16:41
Copy link
Member

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

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

Thanks @KevinGitonga! I think this looks quite solid! I had a bunch of style comments, and a few of substance. PTAL.

@oppiabot
Copy link

oppiabot bot commented Jul 17, 2023

Hi @KevinGitonga, 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 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added stale Corresponds to items that haven't seen a recent update and may be automatically closed. and removed stale Corresponds to items that haven't seen a recent update and may be automatically closed. labels Jul 17, 2023
@oppiabot
Copy link

oppiabot bot commented Jul 26, 2023

Hi @KevinGitonga, 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 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Jul 26, 2023
@adhiamboperes adhiamboperes removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Jul 27, 2023
@oppiabot
Copy link

oppiabot bot commented Aug 3, 2023

Hi @KevinGitonga, 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 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Aug 3, 2023
@adhiamboperes adhiamboperes removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Aug 7, 2023
@oppiabot
Copy link

oppiabot bot commented Aug 14, 2023

Hi @KevinGitonga, 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 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Aug 14, 2023
@adhiamboperes adhiamboperes removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Aug 17, 2023
# Conflicts:
#	app/src/main/java/org/oppia/android/app/home/recentlyplayed/RecentlyPlayedFragmentPresenter.kt
#	app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/learneranalytics/ProfileAndDeviceIdActivityTest.kt
#	app/src/sharedTest/java/org/oppia/android/app/profileprogress/ProfileProgressFragmentTest.kt
@KevinGitonga
Copy link
Contributor Author

Hi @adhiamboperes updated per the comments, might not fully resolve the static failures as i am using my mac which doesn't support bazel used in project last i checked. PTAL thanks

@oppiabot
Copy link

oppiabot bot commented Aug 30, 2023

Hi @KevinGitonga, 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 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added stale Corresponds to items that haven't seen a recent update and may be automatically closed. and removed stale Corresponds to items that haven't seen a recent update and may be automatically closed. labels Aug 30, 2023
@oppiabot
Copy link

oppiabot bot commented Sep 14, 2023

Hi @KevinGitonga, 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 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Sep 14, 2023
@adhiamboperes adhiamboperes removed their assignment Sep 20, 2023
@oppiabot oppiabot bot removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Sep 20, 2023
@oppiabot
Copy link

oppiabot bot commented Sep 27, 2023

Hi @KevinGitonga, 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 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Sep 27, 2023
@oppiabot oppiabot bot closed this Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Corresponds to items that haven't seen a recent update and may be automatically closed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor ProfileId provision process to multiple Activities
3 participants