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 #5195: RestrictedApi. Using a private API #5198

Merged
merged 8 commits into from
Oct 20, 2023

Conversation

sichchurov
Copy link
Collaborator

@sichchurov sichchurov commented Oct 17, 2023

Explanation

Fix part of #5195: I have modified method setButtonTint to avoid an error due to using a restricted API.
This method(bindingAdapter) is not set in any of the layouts. Only test is written for it.
So it is safety to modify the test also. If not modify a test - it is failed then.

Lint report before
callingNewApiBefore

Lint report after
callingNewApiBefore

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)
  • For PRs introducing new UI elements or color changes, both light and dark mode screenshots must be included
  • Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

@sichchurov sichchurov requested a review from a team as a code owner October 17, 2023 21:12
@sichchurov sichchurov marked this pull request as draft October 18, 2023 13:48
@sichchurov sichchurov marked this pull request as ready for review October 19, 2023 06:58
@sichchurov
Copy link
Collaborator Author

@adhiamboperes, PTAL
Thanks

@oppiabot oppiabot bot assigned adhiamboperes and unassigned sichchurov Oct 19, 2023
@oppiabot
Copy link

oppiabot bot commented Oct 19, 2023

Unassigning @sichchurov since a re-review was requested. @sichchurov, please make sure you have addressed all review comments. Thanks!

@adhiamboperes adhiamboperes added the Hacktoberfest This is a suggested Hacktoberfest issue. label Oct 19, 2023
Copy link
Collaborator

@adhiamboperes adhiamboperes left a comment

Choose a reason for hiding this comment

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

Thanks @sichchurov.

The solution looks good, but it is not correct to say that this attribute is unused. I searched, and found multiple usages. See screenshot:
Screenshot 2023-10-20 at 15 56 48

@oppiabot
Copy link

oppiabot bot commented Oct 20, 2023

Unassigning @adhiamboperes since the review is done.

@oppiabot
Copy link

oppiabot bot commented Oct 20, 2023

Hi @sichchurov, it looks like some changes were requested on this pull request by @adhiamboperes. PTAL. Thanks!

@sichchurov
Copy link
Collaborator Author

@adhiamboperes, PTAL
Thanks

@oppiabot oppiabot bot assigned adhiamboperes and unassigned sichchurov Oct 20, 2023
@oppiabot
Copy link

oppiabot bot commented Oct 20, 2023

Unassigning @sichchurov since a re-review was requested. @sichchurov, please make sure you have addressed all review comments. Thanks!

Copy link
Collaborator

@adhiamboperes adhiamboperes 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 @sichchurov!

@oppiabot
Copy link

oppiabot bot commented Oct 20, 2023

Unassigning @adhiamboperes since they have already approved the PR.

@oppiabot oppiabot bot added the PR: LGTM label Oct 20, 2023
@oppiabot
Copy link

oppiabot bot commented Oct 20, 2023

Hi @sichchurov, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to ask someone to merge your PR once the CI checks pass and you're happy with it. Thanks!

@adhiamboperes adhiamboperes merged commit e151bbd into oppia:develop Oct 20, 2023
@sichchurov sichchurov deleted the issue#5195_restricted_api branch October 20, 2023 21:30
Rd4dev pushed a commit to Rd4dev/oppia-android that referenced this pull request Oct 31, 2023
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fix part of oppia#5195: I have modified method setButtonTint to avoid an
error due to using a restricted API.
This method(bindingAdapter) is not set in any of the layouts. Only test
is written for it.
So it is safety to modify the test also. If not modify a test - it is
failed then.

**Lint report before**
<img
src="https://github.com/oppia/oppia-android/assets/71126152/fe9d691b-1bd0-45e9-9f14-7daa73839069"
width="70%" height="70%" alt="callingNewApiBefore">

**Lint report after**
<img
src="https://github.com/oppia/oppia-android/assets/71126152/5689d095-d4ed-4e60-b431-790037b240c3"
width="50%" height="50%" alt="callingNewApiBefore">


<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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
<!-- Delete these section if this PR does not include UI-related
changes. -->
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](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hacktoberfest This is a suggested Hacktoberfest issue. PR: LGTM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants