Skip to content

Addresses lint issues in AGP 7.2.1 update#16839

Merged
ParaskP7 merged 4 commits intoupgrade-gradle-to-7.4.2-agp-to-7.2.1from
agp-7.2.1/lint-issues
Jul 6, 2022
Merged

Addresses lint issues in AGP 7.2.1 update#16839
ParaskP7 merged 4 commits intoupgrade-gradle-to-7.4.2-agp-to-7.2.1from
agp-7.2.1/lint-issues

Conversation

@oguzkocer
Copy link
Copy Markdown
Contributor

@oguzkocer oguzkocer commented Jun 29, 2022

This PR addresses the lint issues due to AGP 7.2.1 update in #16834.

  • 84598d0 removes the unused TextDrawable widget, so we don't even need to fix the lint issue 🙈
  • d1eeb1e removes the custom divider and uses the built in DividerItemDecoration instead. Funnily enough I worked on the people list 6+ years ago and DividerItemDecoration was not available back then (was added a few months after) so we had to build it ourselves. The difference between the regular divider and the custom divider is that the regular divider covers the whole width whereas the custom divider had some space before and after the line. See the screenshots below for comparison. Since we are using the regular divider in a lot of newer screens, this change should bring the design of people list closer to the rest of the app so it's a win-win situation.
  • 9988ff8 addresses Obsolete SDK_INT version checks lint error.

Before:

(Full name and display name are removed and the screenshot was cut in half to make it easier to see)

After:

(Full name and display name are removed and the screenshot was cut in half to make it easier to see)

To test:

  • Smoke test the app
  • Checkout the People page from "My Site" -> "People" and verify that the divider looks OK

Regression Notes

  1. Potential unintended areas of impact
    N/A

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    N/A

  3. What automated tests I added (or what prevented me from doing so)
    N/A

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@oguzkocer oguzkocer added the Lint label Jun 29, 2022
@oguzkocer oguzkocer added this to the 20.3 milestone Jun 29, 2022
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Jun 29, 2022

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@oguzkocer oguzkocer changed the base branch from trunk to upgrade-gradle-to-7.4.2-agp-to-7.2.1 June 29, 2022 16:44
@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Jun 29, 2022

You can test the WordPress changes on this Pull Request by downloading an installable build (wordpress-installable-build-pr16839-0e12fd6.apk), or scanning this QR code:

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Jun 29, 2022

You can test the Jetpack changes on this Pull Request by downloading an installable build (jetpack-installable-build-pr16839-0e12fd6.apk), or scanning this QR code:

@oguzkocer oguzkocer force-pushed the agp-7.2.1/lint-issues branch from 7519752 to 9988ff8 Compare June 29, 2022 17:04
@oguzkocer oguzkocer marked this pull request as ready for review July 4, 2022 15:20
@oguzkocer oguzkocer requested a review from ParaskP7 July 4, 2022 15:20
@ParaskP7 ParaskP7 self-assigned this Jul 5, 2022
Copy link
Copy Markdown
Contributor

@ParaskP7 ParaskP7 left a comment

Choose a reason for hiding this comment

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

👋 @oguzkocer !

I have reviewed and tested this PR as per the instructions, everything works as expected, good job! 🌟


I have left one suggestion (💡) for you to consider. I am going to approve this PR anyway, since none is blocking. I am NOT going to merge this PR yet to give you some time to apply any of my suggestions. However, feel free to ignore them and merge the PR yourself.

@ParaskP7
Copy link
Copy Markdown
Contributor

ParaskP7 commented Jul 5, 2022

Funnily enough I worked on the people list 6+ years ago and DividerItemDecoration was not available back then (was added a few months after) so we had to build it ourselves.

  1. It feels nice to replace code you've written in the past with a much simpler version of it, isn't it! 😄
  2. I find awesome the fact that you keep working and applying love to a codebase for more than 6 years now, kudos! ❤️

@peril-wordpress-mobile
Copy link
Copy Markdown

Warnings
⚠️ PR has more than 300 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

@ParaskP7 ParaskP7 merged commit 5e2660c into upgrade-gradle-to-7.4.2-agp-to-7.2.1 Jul 6, 2022
@ParaskP7 ParaskP7 deleted the agp-7.2.1/lint-issues branch July 6, 2022 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants